Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests: make yarn unit run all available unit tests #13148

Merged
merged 5 commits into from
Sep 30, 2021

Conversation

brendankenny
Copy link
Member

@brendankenny brendankenny commented Sep 30, 2021

we keep growing yarn unit by adding every new yarn unit-*, but jest.config.js has to have references to these files anyways, so yarn jest works just fine if you just want to run all the unit tests :)

This also fixes some overlaps and cracks between the yarn unit-*s. The current yarn unit doesn't run

  • clients/test/extension/popup-test.js
  • clients/test/extension/settings-controller-test.js
  • clients/test/lightrider-entry-test.js
  • docs/recipes/integration-test/example-lh-auth.test.js
  • third-party/chromium-synchronization/inspector-issueAdded-types-test.js
  • third-party/chromium-synchronization/installability-errors-test.js

the last two might be on purpose but seems like they should be in the testing path if the goal is for changes in Canary to alert us to needed updates? It also made us miss that the path to LH_ROOT was wrong in inspector-issueAdded-types-test.js and it doesn't run at all.

We're also running the flow-report tests twice because they're picked up by both yarn unit-report and yarn unit-flow.

This is most important in CI, where unit.yml runs them via yarn unit:ci and should be running all the tests.

@brendankenny brendankenny requested a review from a team as a code owner September 30, 2021 00:45
@brendankenny brendankenny requested review from adamraine and removed request for a team September 30, 2021 00:45
@google-cla google-cla bot added the cla: yes label Sep 30, 2021
@brendankenny
Copy link
Member Author

  • clients/test/extension/popup-test.js
  • clients/test/extension/settings-controller-test.js
  • clients/test/lightrider-entry-test.js
  • docs/recipes/integration-test/example-lh-auth.test.js

well, these are covered by yarn test-clients and yarn test-docs in ci.yml. I'm pretty fine running them with yarn unit if they're going to be listed in the jest.config.js, though.

Context here is that #13146 is adding yet another directory with tests in it, and I don't really have any interest in adding a yarn unit-shared with bonus && yarn unit-shared and && npm run unit-shared -- --ci when yarn unit could just run them all and yarn jest shared works just fine for narrowing to just those tests already :)

@brendankenny
Copy link
Member Author

rather than make docs/recipes/integration-test/example-lh-auth.test.js work in the unit tests, would rather just remove it from the unit tests (#13150)

@brendankenny
Copy link
Member Author

brendankenny commented Sep 30, 2021

updated with docs test changes from #13150 so yarn test-docs doesn't run under yarn unit.

Also moved the *-test-pptr.js tests previously identified as flaky (they have automatic retry in CI) out of the default unit tests. They now only run under their respective yarn test-clients/yarn test-viewer/yarn test-treemap and not under yarn unit, which is exactly the current behavior.

fraggle-rock/**/*-test-pptr.js has been running in unit.yml without flakes for a while now, so seems ok to leave under unit tests (for now?).

@Mbellsudteen
Copy link

Mbellsudteen commented Oct 2, 2021 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants