Determine testing strategies

[Benaiah]

Intro

We have multiple different test systems in the CMS, none of which are very well documented. Some parts of the CMS have no established methods of testing, and others have diverged from their tests. We're at the point where we semi-regularly merge PRs with the CI failing, and there's very little guidance for contributors to improve the situation.

We need to determine, implement, and document test systems for the CMS. We should do this in a way that contributors can help with from the beginning - this will help ensure that our approach works for our community from the start.

The below todos are not final - all of the below is contingent on the decisions we make in discussing this issue (I'll update the issue with our decisions as we make them). The todos are just there to give a rough idea of what's involved in implementing the different pieces of the puzzle.

React testing

Jest

We're using Jest for unit testing our React components - so far I don't see a reason to change this.

Component testing and specs

We're taking a look at revitalizing our Storybook setup for allowing faster component development and spec validation. I met with the Netlify app team to discuss their experience with storybook - they've used it with great success so far. (@bcomnes, @imorente, and @lunaceee, any feedback you have on this issue would be appreciated.)

Todo

• Set up storybook project.
• Get storybook specs running as part of the standard test suite (for CI and command-line use)
• Decouple widget components or mock their dependencies so widgets can be tested in storybook.
• Create stories for current components.
• Document storybook workflow for contributors
• Begin creating a reusable collection of UI elements.

Snapshots

Snapshots haven't provided much insurance, and have mostly just been a hassle for new contributors due to the Git issues they can cause. Should we consider dropping these in favor of Storybook component specs?

Todo:

• Decide whether to keep or drop snapshots
• If kept, improve tooling and documentation to make snapshots easier for contributors to use.

API testing

Testing the APIs has been difficult for a couple reasons:

• The backends are currently stateful objects, which means you have to reinitialize them every time to ensure tests are not coupled to each other.
• Mocking the endpoints means we have to manually recreate and update JSON responses from various services, and doesn't actually test the backends against real responses.
• There's no shared library of common backend functions that can be tested and updated in sync.
• There's no common test suite that can be run against any backend to make sure they're doing equivalent functionality.

Todo

Prep work

• Move backend state into Redux store so backends can be tested statelessly, and provide a state argument and a state-storage function for the backends to store required state information within the Redux store, without coupling the implementations themselves to Redux.
• Separate common backend functions into shared code.
• Document new backend architecture for contributors.

API-specific tests

• Decide if we want to mock endpoints or use live-backend tests for API-specific tests (or not do API-specific tests at all).
• Begin creating API-specific tests.
• Document API-specific testing strategy for contributors.

Common backend test suite

• Determine how to run a common backend test suite (perhaps by pushing redux actions?)
• Determine how to filter tests from inapplicable backends (e.g., not running editorial workflow tests on backends that don't support the editorial workflow).
• Begin creating a common backend test suite.
• Document common backend test suite for contributors

End to end testing

For end-to-end testing, Cypress looks like an interesting option. Unfortunately, it currently only supports Chromium-based browsers, but the test running and creation look very smooth. Personally, I like Cypress for this, as the cross-browser solutions look much less polished, and single-browser E2E testing is better than none at all. Cross-browser functionality is on Cypress' radar, and it's close to being complete for Firefox (cypress-io/cypress#310).

Possible alternatives:

• Selenium?
• Custom headless-chrome solution?
• SAAS solutions?
• your suggestions here

Todo

• Decide on E2E testing strategy.
• Get E2E tests running as part of CI/command-line test suite.
• Begin creating E2E tests.
• Document E2E test workflow.

Code Formatting/Linting

As per #363, we're planning on adding Prettier to the repo to handle code formatting. We already use ESLint, but the config isn't followed very consistently and conflicts with what Prettier uses.

Todo

• Decide whether to use a non-default Prettier format (the only changes I'd personally care for are --trailing-commas all and --print-width <something wider than 80>, and I'd be fine with the defaults as well).
• Integrate prettier with eslint:
  • Make prettier run through eslint --fix
  • Use prettier-compatible eslint config
  • Make eslint check for proper code formatting in test suite.
• Run prettier on codebase and merge.

Coverage

The changes to our tests will likely necessitate improvements to our code coverage system. Mostly, we'll need to ensure that coverage includes the different types of tests we're looking to add.

CI

As we get a better testing scheme, we should re-emphasize the role of our CI system in development. Adding the above tests to our main test suite will increase the utility of our CI system a lot, but we may not want to block PR merges in all cases:

• Should we block on code coverage reduction? TBD
  I suggest we don't do this - it seems likely to frustrate contributors for little benefit.
• Should we block on incorrect code formatting? TBD
  I suggest we do so, since bad code formatting can be fixed automatically.
• Should we block on snapshot failures? TBD
  I suggest we remove snapshots entirely.
• Should we block on storybook spec failures? TBD
  I suggest we do so.

Developer Accessibility

I've noted this already in the "Todo" lists, but all this needs to be very accessible to contributors if we want any uptake. Testing is already a bit of a burden for many contributors, and having to figure out the test systems we use and where they apply without clear guidance multiplies that.

Summary

Please leave any thoughts you have on the above. As we decide on different aspects of this issue, we'll likely split them off into their own issues for status tracking and more detailed discussion.

[Benaiah]

@erquhart, @tech4him1 ☝️

[jeddy3]

I hope you don't mind me chiming in on this issue. I've some thoughts on the Code Formatting/Linting section that I'd like to share.

We recently started using the Netlify CMS at the children's charity where I work. It was fantastically easy to set up and the development team has been very impressed with it! So, I'd like to find some way to give back to this project.

I'm the co-creator of stylelint. It's the linter currently being used to lint the CSS of the Netlify CMS UI. I've spent a significant chunk of the last few years configuring, thinking about and working with linters and pretty-printers. Over that time, I believe myself (and the other stylelint contributors) have focused in on a few practices that work well. I'd like to share those now, in case you find them useful.

The CSS-Tricks' Prettier + Stylelint: Writing Very Clean CSS (Or, Keeping Clean Code is a Two-Tool Game) article does a great job of capturing the current landscape. The article is focused on CSS code, but the message is applicable to JavaScript code too. It can be summarised as, use:

• pretty-printers to format your code
• linters to check your code for mistakes and enforce non-stylistic conventions

With that in mind, here are some thoughts related to the Code Formatting/Linting section.

Decide whether to use a non-default Prettier format (the only changes I'd personally care for are --trailing-commas all and --print-width <something wider than 80>, and I'd be fine with the defaults as well).

The Prettier docs recommend using the default options, and they recommend against using a width wider than 80. I believe that the algorithm that decides when to break a line is optimised for 80 characters.

Within teams I've worked with, we've also found sticking entirely with the default options makes it easier for us to setup Prettier in our individual code editors. This is also beneficial when onboarding external contributors.

Make prettier run through eslint --fix [or stylelint --fix]

We've found that this approach can lead to actual lint errors being lost in a sea of formatting warnings. Using Prettier as part of a precommit hook avoids this problem. This is how we use Prettier to pretty-print the JavaScript code of stylelint.

Use prettier-compatible eslint config

ESLint comes with a recommended config ("extends": "eslint:recommended") which contains just rules that check for mistakes and enforce non-stylistic conventions. I believe all these rules are compatible with Prettier. We recently followed suit at stylelint and released stylelint-config-recommended.

I'd recommend starting with just these recommended configs (when using Prettier) and then layering on additional rules as you need them. We took this approach with the stylelint ESLint config when we adopted Prettier and it's worked well this past year.

// .stylelintrc
{ 
  "extends": "stylelint-config-recommended"
}

// .eslintrc
{ 
  "extends": "eslint:recommended",
  "env": {
    "es6": true,
    "browser": true,
    "node": true,
    "jest": true,
  }
  ..
}

Make eslint check for proper code formatting in test suite.

The precommit hook should ensure that the code is correctly pretty-printed. However, Prettier has a --list-different flag, which can be a safety net on the CI. We use it in stylelint.

---

I hope some of the above proves helpful. If you like, feel free to at-mention me in any PRs related to stylelint and I'll make the time to take a look. Thanks again for creating a fantastic open source CMS!

As an aside, I'd be more than happy to help optimise your .stylelintrc file once you've finished the process of adopting Prettier. For example, there's a stylelint plugin, stylelint-selector-bem-pattern, that can enforce your BEM naming conventions. This can be particularly useful as writing BEM code can be quite tricky for developers/contributors unfamiliar with its nuances.

[erquhart]

@jeddy3 great breakdown, thanks for taking the time to write it. We're about to get Prettier running in the above linked PR, #952, happy to have any improvements you'd like to contribute!

We're doing pretty much what you said in terms of config, trailing commas set to "all", but with the default line width. I did consider moving to 100 characters, which is what we loosely an unofficially do currently.

Regarding BEM, we don't actually use it, and that's intentional, we use more of a lazy, hyphenated subset. Using raw CSS isn't our final destination, just an interim strategy until a defacto CSS-in-JS approach is well adopted. I do have my eye pretty squarely on styled-components as of late.

Again, thank you, and contributions are more than welcome!

[jeddy3]

I do have my eye pretty squarely on styled-components as of late.

Yes, styled-components do feel like a strong contender within the CSS-in-JS space!

Prettier can already pretty-print the CSS within the template literals of a styled-component. And styled-components itself uses stylelint to lint the CSS within the template literals. So, with styled-components we can still have the best of both worlds:

• a pretty-printer to format our CSS code and free us from having to deal with stylistic concerns
• a linter to:
  • check our CSS code for mistakes and surface bugs earlier (e.g. overriding shorthand properties)
  • enforce non-stylistic conventions and make our code CSS more consistent (e.g. limiting what units can be used)

The Prettier and stylelint combo will work in the interim and will continue to work if/when you adopt styled-components.

Again, thank you, and contributions are more than welcome!

Thanks for the welcoming invitation! Like I mentioned above, I'm more than happy to help. I'll keep an eye out for an opportunity to help optimise the config files for either or both of these scenarios :)

[talves]

I think https://github.com/kentcdodds/react-testing-library might be worth a look.

I really liked how this looks: https://codesandbox.io/s/github/kentcdodds/react-testing-library-examples

[tomrutgers]

@talves react-testing-library is being used already ^^
(source)

[talves]

ok, there we go! 😜 Now we just need to really use it 😀

I see how I missed it now.

[lunaceee]

This looks great! @Benaiah One lil note regarding Snapshot testing, we do try to avoid doing it in the app as it seems to be mostly implementation details.

[erquhart]

Agreed, react-testing-library can help us avoid that. A couple of updates:

I've had some success with docz as a simpler alternative to Storybook with a lot of powerful, batteries included functionality, such as their playground component for live editing components. Storybook would allow us to test stories, but conflating testing and docs in this manner can lead to really ungainly documentation. The real challenge is setting up UI components to work outside of the CMS. We still have a lot of work to do toward decoupling distinct areas of the application.

Lastly, I also have a fairly pleasant backend testing approach working locally using nock to mock requests (rather than mocking the backend request method). Rather than testing the backend direct, we test them through a partially mocked backend.js, which provides the actual API that the rest of the CMS consumes. This approach allows us to skip unit testing, which will tie us to implementation details, and focus instead on testing the integration. Currently working on completely covering the GitLab backend as a prerequisite to merging #1817. We should be able to arrive at a scenario where decoupled API mocks can be used for both integration testing and with our Cypress e2e.

More coming soon, these are current priorities.

_{Sent with GitHawk}

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[erquhart]

@erezrokah as you've done some extensive work on our testing strategy, feel free to update or close this.

[erezrokah]

@erquhart I think it's worth separating this into different issues. I marked a lot of the checkboxes related to API and E2E testing (and linting/formatting), though I'm not sure I understand what does "moving backends state to redux" means.

I think we've pretty much settled with Cypress for e2e testing (especially now that they have headless Chrome support).

After the BitBucket+GitLab editorial workflow feature is merged we'll have a shared editorial workflow test for BitBucket, GitLab and GitHub backends, a separate one for the Test backend and open authoring tests for GitHub.

For the media library we'll have a shared spec for all the backends (even for git-gateway with Netlify Large Media support).

All e2e tests use pre-recorded data and the process of recording is documented (while not that straightforward).

Not sure about Unit Testing specific API methods or integration tests as I don't thing they'll provide much confidence with regards to the backends.

As long as we can parallelize the tests to not increase CI time I'm good with the current approach (anyway the big chunk of our CI time is the build process and not the tests).

[erezrokah]

Closing as done