Setup Playwright tests, misc fixes

vincerubinetti commented 3 months ago

Note that the diff here is a pain because I had to do a bulk chmod due to a playwright setup command messing with a bunch of file permissions, and couldn't find a way to reset the permissions to their defaults (pre-playwright-command) without going through one file at a time.

pull in various improvements from https://github.com/krishnanlab/geneplexus-app-v2/pull/37 and https://github.com/krishnanlab/geneplexus-app-v2/pull/38
update test workflow: fix trigger, update action versions, update debugging steps, add e2e playwright testing with caching
setup playwright for e2e and accessibility testing
add status indicator to analysis card component
enable msw network request mocking
don't allow empty "lookup analysis"
install and setup axe-core (library that powers chrome dev tools' lighthouse accessibility check)
check all major pages of app (so far) for accessibility issues
add basic e2e test for demonstration

netlify[bot] commented 3 months ago

Deploy Preview for molevolvr ready!

Name	Link
Latest commit	ff5b1a8c5a6f0195633ce1f759377ce16e94bc40
Latest deploy log	https://app.netlify.com/sites/molevolvr/deploys/66cf5eb0ca2cc90008668819
Deploy Preview	https://deploy-preview-4--molevolvr.netlify.app
Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

vincerubinetti commented 3 months ago

I've just added a commit that pulls in updates from https://github.com/krishnanlab/geneplexus-app-v2/pull/38, which includes unrelated fixes, but also some accessibility fixes that would affect the Axe testing here. It also adds /testbed to the routes that are tested with Axe.

falquaddoomi commented 2 months ago

Hey, do you expect people to run these tests you've added locally, or are they only for the GitHub action? If I can run them locally, how do I do that? I'm getting the following when I try to do what seems obvious from reading the scripts you've defined in package.json:

[ faisala@gleipnir molevolvr2.0/app ] % bun run install-playwright
$ bunx playwright install --with-deps
[ faisala@gleipnir molevolvr2.0/app ] % bun run test:e2e                  
[WebServer] $ vite --port 1234

SyntaxError: Unexpected token 'with'

SyntaxError: Unexpected token 'with'

Error: No tests found

To open last HTML report run:

  npx playwright show-report

error: script "test:e2e" exited with code 1
[ faisala@gleipnir molevolvr2.0/app ] % bun run test    
$ bun run test:lint && bun run test:types && bun run test:e2e
$ eslint . && prettier **/*.{tsx,ts,css,html,md,json,yaml} --check --no-error-on-unmatched-pattern

Oops! Something went wrong! :(

ESLint: 9.8.0

ConfigError: Config (unnamed): Key "rules": Key "constructor-super": structuredClone is not defined
    at rethrowConfigError (/Users/faisala/Checkouts/chai/jravilab/molevolvr2.0/app/node_modules/@eslint/config-array/dist/cjs/index.cjs:303:8)
    at /Users/faisala/Checkouts/chai/jravilab/molevolvr2.0/app/node_modules/@eslint/config-array/dist/cjs/index.cjs:1098:5
    at Array.reduce (<anonymous>)
    at FlatConfigArray.getConfigWithStatus (/Users/faisala/Checkouts/chai/jravilab/molevolvr2.0/app/node_modules/@eslint/config-array/dist/cjs/index.cjs:1091:43)
    at FlatConfigArray.getConfig (/Users/faisala/Checkouts/chai/jravilab/molevolvr2.0/app/node_modules/@eslint/config-array/dist/cjs/index.cjs:1120:15)
    at /Users/faisala/Checkouts/chai/jravilab/molevolvr2.0/app/node_modules/eslint/lib/eslint/eslint-helpers.js:346:56
    at Array.reduce (<anonymous>)
    at /Users/faisala/Checkouts/chai/jravilab/molevolvr2.0/app/node_modules/eslint/lib/eslint/eslint-helpers.js:333:36
    at /Users/faisala/Checkouts/chai/jravilab/molevolvr2.0/app/node_modules/eslint/lib/eslint/eslint-helpers.js:296:32
    at Object.isAppliedFilter (/Users/faisala/Checkouts/chai/jravilab/molevolvr2.0/app/node_modules/@nodelib/fs.walk/out/readers/common.js:12:31)
[ faisala@gleipnir molevolvr2.0/app ] %

vincerubinetti commented 2 months ago

Yes you should be able to run them locally. Do you have node v22 installed?

falquaddoomi commented 2 months ago

Aha; I wasn't using node v22 at the time, but I switched to v22.7.0, wiped out node_modules, reinstalled using bun install, and ran bun run test with the following (successful?) output:

% bun run test
$ bun run test:lint && bun run test:types && bun run test:e2e
$ eslint . && prettier **/*.{tsx,ts,css,html,md,json,yaml} --check --no-error-on-unmatched-pattern
Checking formatting...
All matched files use Prettier code style!
$ tsc
[WebServer] $ vite --port 1234

(node:9206) ExperimentalWarning: Importing JSON modules is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)

Running 27 tests using 5 workers
(node:9279) ExperimentalWarning: Importing JSON modules is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
(node:9282) ExperimentalWarning: Importing JSON modules is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
(node:9278) ExperimentalWarning: Importing JSON modules is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
(node:9281) ExperimentalWarning: Importing JSON modules is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
(node:9280) ExperimentalWarning: Importing JSON modules is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
(node:9455) ExperimentalWarning: Importing JSON modules is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
(node:9456) ExperimentalWarning: Importing JSON modules is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
(node:9485) ExperimentalWarning: Importing JSON modules is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
(node:9486) ExperimentalWarning: Importing JSON modules is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
(node:9487) ExperimentalWarning: Importing JSON modules is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
[webkit] › home.spec.ts:4:1 › Example analyses show
(node:9557) ExperimentalWarning: Importing JSON modules is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
(node:9556) ExperimentalWarning: Importing JSON modules is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
(node:9579) ExperimentalWarning: Importing JSON modules is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
(node:9584) ExperimentalWarning: Importing JSON modules is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
  16 skipped
  11 passed (9.6s)

To open last HTML report run:

  npx playwright show-report

Perhaps you should specify the version of node that's required via https://docs.npmjs.com/cli/v10/configuring-npm/package-json#engines, or some other way like in the README?

Also, adding something to your PR description that specifies what you'd like me to test and how to do it would be nice.

vincerubinetti commented 2 months ago

Sorry, I thought that was already in the readme. I've updated the readme to add some clarifications.

Also, adding something to your PR description that specifies what you'd like me to test and how to do it would be nice.

What I can do from now on is make a comment near sections of the diff that are particularly complex/error-prone/high-consequence. I did this for monarch and it helped them ignore irrelevant changes. I will do this now for this PR.

falquaddoomi commented 2 months ago

What I can do from now on is make a comment near sections of the diff that are particularly complex/error-prone/high-consequence. I did this for monarch and it helped them ignore irrelevant changes. I will do this now for this PR.

Thanks; I realize this took effort, and it is helpful. What I meant, though, was just some simple directions for things you'd want tried out by a reviewer, e.g. "Run bun run test and see that the tests complete" or "try out feature X in widget Y". I'm not much of a frontend dev anymore, and just simple things like "use node version X" or "run this command to launch the test suite" would save me a little time.

You're of course welcome to continue to describe the significant code changes in your PR and why you made them, but I don't expect you to.

vincerubinetti commented 2 months ago

Gotcha. I'll still try to do the comment-by-comment when possible, still seems helpful on large PRs so you can sort of skip over lower risk/impact things. And if not I'll have a paragraph or so summarizing the salient things.

falquaddoomi commented 2 months ago

Just FYI, I've looked through the changes and I still approve it all. Feel free to merge.

JRaviLab / molevolvr2.0