Keystone: Set up tests

[JedWatson]

@timleslie assigned this one to you, but feel free to pass it on if you want.

From the top of my head, the test processes we'll want to add are:

• ESLint
• Unit Tests (functional, per package)
  
  
  
  • Cover things like creating a new Keystone instance, adding Lists, retrieving meta and GraphQL config
  
  
  • Creating a new Admin UI and WebServer
  
  
• Field Type Unit Tests (based on what was set up for Keystone 4)
  
  
  
  • Any functional tests against a field type (e.g. formatting, validation, etc)
  
  
  • Seed a generated list with data colocated with the field code, spin up a GraphQL endpoint and assert filters work as expected
  
  
• Integration Tests (initialise test project w/ seed database, use cypress to test against it)
  
  
  
  • Should run through basic nav, check all views load
  
  
  • Search, add columns, etc. in the List view
  
  
  • Add / edit / delete an Item
  
  

We should probably also add some tests for the UI package - maybe Jest based? cc/ @jossmac lmk what you think would be helpful here

While things are in a really prototypey stage I don't think we should be aiming for comprehensive test coverage, but getting all the infrastructure in place would be really useful so we can add tests for different areas as they stabilise, and having smoke tests asap would be really valuable because we're starting to get quite a bit of surface area and manual testing in the Admin UI can easily miss stuff.

[JedWatson]

Another idea: since we're using prettier for this project, but it's not enforced in a pre-commit hook (because that causes issues with partial commits) it would be nice to run prettier against the whole codebase in CI and assert nothing has changed.

That would make it really easy to stop commits coming in that pass listing but aren't actually formatted by prettier before they are pushed.

[jesstelford]

My experience with running prettier (or other linters) in CI has been less than satisfying: By the time the code is in a PR, as a contributor I don't want to get blocked because CI says the code isn't formatted correctly even though all the tests may have passed locally, etc.

it's not enforced in a pre-commit hook (because that causes issues with partial commits)

What kinds of issues have you run into? I'm a fan of having lint-staged execute prettier-eslint on pre-commit. While it makes commits noisier, it has an escape hatch (--no-verify), and means I know immediately when a build might fail. For example: https://github.com/jesstelford/scripts/blob/master/package.json#L57-L60

[jesstelford]

Cover things like creating a new Keystone instance, adding Lists, retrieving meta and GraphQL config

Seed a generated list with data colocated with the field code, spin up a GraphQL endpoint and assert filters work as expected

Agreed! To put it another way:

1. Test that the _correct graphQL schema is generated_, and
2. Test that the _resolvers of the schema work as expect_ 👍

[lukebatchelor]

The only real issue is that it will affect all changed files during a commit, not just the staged chunks, so it ends up staging the whole file after it runs.

Personally this doesn't affect my workflow too much as I'm usually squashing all my work into one commit any way.

[JedWatson]

@jesstelford maybe I've misunderstood how that works. My bad experience is only with automatically formatting code with prettier and then doing a blind git add, which breaks when you've only staged _some_ of the changed parts of a file.

If your solution can report that prettier _needs to be run_ as part of the pre-commit hook, I'm all for that 👍

[JedWatson]

btw there's plenty to do here, @jesstelford / @lukebatchelor (or anyone else) feel free to pick up any of the items I've listed above if you're keen. Maybe we should use this as a roll-up issue and create granular issues for the individual tasks _as someone starts on them_ so it's easier to stay in sync with each other?

[jesstelford]

Sorry to bike shed! Looking at my code again, I realise I don't do a git add, but only run the lint check to report if anything is out of whack. Then I manually run the lint:fix command, and rerun git add -p.

Definitely adds more steps to committing!

Arguably if those steps are going to have to happen regardless, better to do it at commit time, rather than after PR > CI > check email notifications, by which point I've context switched out and motivation to restart is low.

[timleslie]

As a first step in this I'm currently working on a PR which will

• Add lint and lint_ci targets to all package.json files to allow for easy linting both locally and on the CI server.
• Add a circleCi config to run the lint_ci script.

Should be good to go by the end of the night unless I hit something weird.

Edit:

Discussed this with Jed and decided to go with just a top-level lint and lint_ci script to keep things simple. The minor changes required for this are in PR https://github.com/keystonejs/keystone-5/pull/36. You can follow the first commit on that branch to see what circle CI looks like when it identifies a lint error.

[dominikwilkowski]

Let's keep track of what we got so far:

• [x] ESLint
• [x] Unit Tests
  
  
  
  • [x] Unit Tests per package
  
  
  • [x] Cover things like creating a new Keystone instance, adding Lists, retrieving meta and GraphQL config
  
  
  • [x] Creating a new Admin UI and WebServer
  
  
• [x] Field Type Unit Tests (based on what was set up for Keystone 4)
  
  
  
  • [x] Any functional tests against a field type (e.g. formatting, validation, etc)
  
  
  • [x] Seed a generated list with data colocated with the field code, spin up a GraphQL endpoint and assert filters work as expected
  
  
• [x] Integration Tests
  
  
  
  • [x] Initialise test project w/ seed database, use cypress to test against it
  
  
  • [x] Should run through basic nav, check all views load
  
  
  • [x] Field tests
  
  
  • [x] Search in the List view
  
  
  • [x] Add columns in the List view
  
  
  • [x] Add an Item
  
  
  • [x] Edit an Item
  
  
  • [x] Delete an Item
  
  
  • [x] Filter tests
  
  
  • [ ] Login test
  
  

[jesstelford]

We are thoroughly tested now 👍