Spike: Testing Improvements

acrollet commented 3 years ago

User Story or Problem Statement

As a developer, I need tests to be:

easy to develop and validate
not affect content that CMS end users will see
use a modern browser environment
total test run time is as long as the longest test

Acceptance Criteria

Participants

Required: @indytechcook @swirtSJW @kaise-lafrai @ndouglas @ethanteague @acrollet @ElijahLynn @olivereri Optional: @mchelen-gov

[ ] Spike is held and actionable next steps and implementation timeline are generated
The following issues are addressed
- tests are run against the same DB that users see
- development workflow is improved
- modern browser environment should be used for functional tests
[ ] A process is defined for ensuring that new content types are tested for accessibility as they are created

Implementation steps

Possible solutions to discuss:

End to End/Functional Tests complete within 10 minutes of push
Using cypress for all functional testing
Test against an in-memory sqlite copy of the DB (perhaps purged of as much content as possible)
When and under what conditions is test-generated content removed? (e.g. at the end of successful tests, at the end of successful tests and when the tests fail, only on specific environments, never)
should we prioritize moving tests to github actions? If so, how & when?
is it worth using a different task runner suite? (go-task vs yaml-tasks, see comment below)

acrollet commented 3 years ago

An alternative to forking yaml-tests or moving to yaml-tests is to use an entirely different test runner suite. There are not a lot of options out there for flexible test runners, but I believe I've found a good one: task

Positives:

Negatives:

no native support for reporting check status/comments to the github api. We would need to use something like reviewdog, github-status-updater, or github-commenter on each step or build a tool if that won't work
must be installed/updated like other OS dependencies, rather than with composer (There are .deb packages provided, tested working on lando)

At a glance, quite a few of our checks can be parallelized, so we should be able to shave some time off the testing process using a tool like this...

ndouglas commented 3 years ago

Thoughts so far re: Cypress:

Cypress doesn't use conventional Promises and isn't by default cleanly integrated with Promises, but it's thoroughly asynchronous, so there's a learning curve. The pain points should be found and exposed in comments, docs, etc to ease onboarding.
Regular expressions seem not to be working (or at least capturing) in step definitions. Not sure where the issue is -- cypress-cucumber-preprocessor, cucumber-expressions, my own idiocy, etc. This contributes to stilted and/or ungrammatical steps, wastes developer time, causes frustration and distraction, etc.
There may or may not be some subtle issues with how callbacks function in some of the Cypress functions, and this should be investigated. (Above and beyond the idiosyncrasies with Cypress' async structure noted above, I feel like there are inconsistencies within the system that may be bugs, and at the very least are subtle and confusing.)
There may or may not be some subtle differences between how Cypress.$/cy.$$ and how window.jQuery function, and this should be investigated. (This might just be my error, but I feel like cy.$$('.some-class') and window.jQuery('.some-class') should consistently resolve to the same things, and they do not. This kills the ~crab~ productivity.)
The existing step definitions need to be refactored heavily, and as much code as possible should be separated into Cypress commands or some other common repository to promote reuse and brevity and clarity of tests.
Strange issues can be encountered when the test doesn't wait between steps -- for instance, without a wait before submitting a form, the form data may not be populated in line with the tester's expectations and the test may fail in an unintuitive fashion.
The functionality within the existing Behat contexts needs to be ported to Cypress.
Following the above, the existing Behat functional tests need to be ported to Cypress.
We probably need a style guide, whether explicit or implicit.
Some tests can pass 98+% of the time but fail randomly, and it's difficult to know when this is the result of developer error vs. irreducible complexity of simulating human interaction on a full-blown web browser

And other things:

Release failed because of an issue in a periodic task that failed because of a module update. As Elijah points out:

Yeah, this isn't the first time periodic has snuck by testing. We should probably run these as a test. But we can't currently because it actually affects something...

This ties in to a recurring issue with side-effect-ful tests and the issues that they frequently cause.

We should ensure that source files (e.g. SASS/SCSS, ES, etc) and their generated derivatives that are kept in VCS (CSS, JS, etc) are in sync. This would prevent some PR false alarms like in #4884 that Adrian had to work around, make testing more reliable, etc.

oksana-c commented 3 years ago

acrollet commented 3 years ago

department-of-veterans-affairs / va.gov-cms