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

(refactor) Refactor test setup to leverage caching by turbo #764

Merged
merged 5 commits into from
Jul 27, 2023

Conversation

denniskigen
Copy link
Member

@denniskigen denniskigen commented Jul 24, 2023

Requirements

Summary

This PR refactors the test setup in this monorepo to leverage the caching capabilities of turbo. The current setup does properly leverage turbo for the following reasons:

  • It uses a single Jest configuration, which gets used by the test scripts in the root-level package.json to run tests across the entire monorepo. As such, all the tests get re-run regardless of whether the associated files changed.

Ideally, we would like to leverage caching when executing tests. This PR introduces exactly that by moving testing-related scripts from the root-level package.json to each package.json file of each package. Additionally, this PR also:

  • Moving mocks from the root-level __mocks__ directory to package-specific __mocks__ directories. This step also involves moving shared mocks to the test-helpers file.
  • Cleaning up a bunch of test warnings and adding some useful refactors.

With these changes, we can now fully leverage caching when running tests.

Video

test-caching-turbo.mp4

Related Issue

None

Other

None

@github-actions
Copy link
Contributor

github-actions bot commented Jul 24, 2023

Size Change: +9 B (0%)

Total Size: 2.12 MB

ℹ️ View Unchanged
Filename Size Change
packages/esm-active-visits-app/dist/130.js 131 kB 0 B
packages/esm-active-visits-app/dist/316.js 42.9 kB 0 B
packages/esm-active-visits-app/dist/319.js 543 B 0 B
packages/esm-active-visits-app/dist/574.js 543 B 0 B
packages/esm-active-visits-app/dist/588.js 6.66 kB 0 B
packages/esm-active-visits-app/dist/629.js 7.48 kB 0 B
packages/esm-active-visits-app/dist/757.js 637 B 0 B
packages/esm-active-visits-app/dist/784.js 2.63 kB 0 B
packages/esm-active-visits-app/dist/788.js 540 B 0 B
packages/esm-active-visits-app/dist/807.js 856 B 0 B
packages/esm-active-visits-app/dist/833.js 660 B 0 B
packages/esm-active-visits-app/dist/842.js 881 B 0 B
packages/esm-active-visits-app/dist/879.js 2.94 kB 0 B
packages/esm-active-visits-app/dist/909.js 54.2 kB 0 B
packages/esm-active-visits-app/dist/936.js 7.96 kB 0 B
packages/esm-active-visits-app/dist/main.js 3.39 kB 0 B
packages/esm-active-visits-app/dist/openmrs-esm-active-visits-app.js 3.22 kB 0 B
packages/esm-appointments-app/dist/130.js 131 kB 0 B
packages/esm-appointments-app/dist/319.js 1.73 kB 0 B
packages/esm-appointments-app/dist/328.js 11.5 kB +6 B (0%)
packages/esm-appointments-app/dist/381.js 10.5 kB -5 B (0%)
packages/esm-appointments-app/dist/437.js 2.44 kB 0 B
packages/esm-appointments-app/dist/456.js 661 B 0 B
packages/esm-appointments-app/dist/469.js 6.65 kB 0 B
packages/esm-appointments-app/dist/574.js 1.72 kB 0 B
packages/esm-appointments-app/dist/591.js 16.9 kB 0 B
packages/esm-appointments-app/dist/610.js 6.71 kB 0 B
packages/esm-appointments-app/dist/738.js 144 kB 0 B
packages/esm-appointments-app/dist/752.js 30.9 kB +23 B (0%)
packages/esm-appointments-app/dist/757.js 1.73 kB 0 B
packages/esm-appointments-app/dist/781.js 103 kB 0 B
packages/esm-appointments-app/dist/784.js 2.63 kB 0 B
packages/esm-appointments-app/dist/788.js 1.73 kB 0 B
packages/esm-appointments-app/dist/80.js 5.09 kB -3 B (0%)
packages/esm-appointments-app/dist/807.js 2.42 kB 0 B
packages/esm-appointments-app/dist/833.js 2.02 kB 0 B
packages/esm-appointments-app/dist/884.js 1.96 kB 0 B
packages/esm-appointments-app/dist/main.js 5.49 kB 0 B
packages/esm-appointments-app/dist/openmrs-esm-appointments-app.js 3.18 kB 0 B
packages/esm-outpatient-app/dist/119.js 27.6 kB -17 B (0%)
packages/esm-outpatient-app/dist/121.js 4 kB +10 B (0%)
packages/esm-outpatient-app/dist/130.js 131 kB 0 B
packages/esm-outpatient-app/dist/176.js 5.94 kB -2 B (0%)
packages/esm-outpatient-app/dist/242.js 218 B 0 B
packages/esm-outpatient-app/dist/252.js 2.21 kB 0 B
packages/esm-outpatient-app/dist/286.js 2.88 kB 0 B
packages/esm-outpatient-app/dist/308.js 8.14 kB -2 B (0%)
packages/esm-outpatient-app/dist/319.js 3.06 kB 0 B
packages/esm-outpatient-app/dist/328.js 3.1 kB 0 B
packages/esm-outpatient-app/dist/330.js 8.61 kB 0 B
packages/esm-outpatient-app/dist/364.js 2.02 kB 0 B
packages/esm-outpatient-app/dist/425.js 3.43 kB 0 B
packages/esm-outpatient-app/dist/457.js 2.49 kB 0 B
packages/esm-outpatient-app/dist/469.js 6.65 kB 0 B
packages/esm-outpatient-app/dist/521.js 2.74 kB 0 B
packages/esm-outpatient-app/dist/53.js 3.11 kB 0 B
packages/esm-outpatient-app/dist/574.js 3.08 kB 0 B
packages/esm-outpatient-app/dist/591.js 16.9 kB 0 B
packages/esm-outpatient-app/dist/610.js 6.71 kB 0 B
packages/esm-outpatient-app/dist/611.js 2.64 kB 0 B
packages/esm-outpatient-app/dist/616.js 3.61 kB +27 B (+1%)
packages/esm-outpatient-app/dist/644.js 1.24 kB 0 B
packages/esm-outpatient-app/dist/670.js 155 kB 0 B
packages/esm-outpatient-app/dist/733.js 4.09 kB 0 B
packages/esm-outpatient-app/dist/757.js 3.06 kB 0 B
packages/esm-outpatient-app/dist/764.js 5.25 kB 0 B
packages/esm-outpatient-app/dist/766.js 4.73 kB 0 B
packages/esm-outpatient-app/dist/784.js 2.63 kB 0 B
packages/esm-outpatient-app/dist/788.js 3.06 kB 0 B
packages/esm-outpatient-app/dist/807.js 4.43 kB 0 B
packages/esm-outpatient-app/dist/833.js 3.66 kB 0 B
packages/esm-outpatient-app/dist/86.js 4.42 kB 0 B
packages/esm-outpatient-app/dist/981.js 2.91 kB 0 B
packages/esm-outpatient-app/dist/main.js 3.96 kB 0 B
packages/esm-outpatient-app/dist/openmrs-esm-outpatient-app.js 3.19 kB 0 B
packages/esm-patient-list-app/dist/130.js 131 kB 0 B
packages/esm-patient-list-app/dist/255.js 4.4 kB 0 B
packages/esm-patient-list-app/dist/294.js 3.29 kB 0 B
packages/esm-patient-list-app/dist/319.js 943 B 0 B
packages/esm-patient-list-app/dist/404.js 4.36 kB 0 B
packages/esm-patient-list-app/dist/469.js 95.9 kB 0 B
packages/esm-patient-list-app/dist/537.js 534 B 0 B
packages/esm-patient-list-app/dist/563.js 15.4 kB 0 B
packages/esm-patient-list-app/dist/574.js 942 B 0 B
packages/esm-patient-list-app/dist/588.js 6.66 kB 0 B
packages/esm-patient-list-app/dist/591.js 16.9 kB 0 B
packages/esm-patient-list-app/dist/716.js 4.62 kB 0 B
packages/esm-patient-list-app/dist/757.js 1.09 kB 0 B
packages/esm-patient-list-app/dist/784.js 2.63 kB 0 B
packages/esm-patient-list-app/dist/788.js 941 B 0 B
packages/esm-patient-list-app/dist/791.js 1.72 kB 0 B
packages/esm-patient-list-app/dist/807.js 1.37 kB 0 B
packages/esm-patient-list-app/dist/833.js 1.13 kB 0 B
packages/esm-patient-list-app/dist/900.js 7.66 kB 0 B
packages/esm-patient-list-app/dist/main.js 6.08 kB 0 B
packages/esm-patient-list-app/dist/openmrs-esm-patient-list-app.js 3.18 kB 0 B
packages/esm-patient-registration-app/dist/130.js 131 kB 0 B
packages/esm-patient-registration-app/dist/167.js 27.4 kB -24 B (0%)
packages/esm-patient-registration-app/dist/177.js 57.2 kB 0 B
packages/esm-patient-registration-app/dist/294.js 23.4 kB 0 B
packages/esm-patient-registration-app/dist/319.js 1.36 kB 0 B
packages/esm-patient-registration-app/dist/537.js 3.74 kB 0 B
packages/esm-patient-registration-app/dist/574.js 1.43 kB 0 B
packages/esm-patient-registration-app/dist/591.js 16.9 kB 0 B
packages/esm-patient-registration-app/dist/62.js 6.86 kB 0 B
packages/esm-patient-registration-app/dist/635.js 456 B 0 B
packages/esm-patient-registration-app/dist/68.js 9.48 kB 0 B
packages/esm-patient-registration-app/dist/735.js 464 B 0 B
packages/esm-patient-registration-app/dist/742.js 721 B 0 B
packages/esm-patient-registration-app/dist/757.js 1.7 kB 0 B
packages/esm-patient-registration-app/dist/784.js 2.64 kB 0 B
packages/esm-patient-registration-app/dist/788.js 1.36 kB 0 B
packages/esm-patient-registration-app/dist/807.js 2.16 kB 0 B
packages/esm-patient-registration-app/dist/833.js 1.66 kB 0 B
packages/esm-patient-registration-app/dist/857.js 2.08 kB 0 B
packages/esm-patient-registration-app/dist/879.js 2.94 kB 0 B
packages/esm-patient-registration-app/dist/9.js 12.2 kB 0 B
packages/esm-patient-registration-app/dist/947.js 28.9 kB 0 B
packages/esm-patient-registration-app/dist/975.js 425 B 0 B
packages/esm-patient-registration-app/dist/main.js 21.8 kB 0 B
packages/esm-patient-registration-app/dist/openmrs-esm-patient-registration-app.js 3.23 kB 0 B
packages/esm-patient-search-app/dist/110.js 16.9 kB -4 B (0%)
packages/esm-patient-search-app/dist/128.js 2.02 kB 0 B
packages/esm-patient-search-app/dist/130.js 131 kB 0 B
packages/esm-patient-search-app/dist/262.js 29.4 kB 0 B
packages/esm-patient-search-app/dist/319.js 779 B 0 B
packages/esm-patient-search-app/dist/327.js 1.05 kB 0 B
packages/esm-patient-search-app/dist/357.js 5.72 kB 0 B
packages/esm-patient-search-app/dist/519.js 5.08 kB 0 B
packages/esm-patient-search-app/dist/574.js 792 B 0 B
packages/esm-patient-search-app/dist/588.js 6.66 kB 0 B
packages/esm-patient-search-app/dist/591.js 16.9 kB 0 B
packages/esm-patient-search-app/dist/757.js 923 B 0 B
packages/esm-patient-search-app/dist/784.js 2.63 kB 0 B
packages/esm-patient-search-app/dist/788.js 778 B 0 B
packages/esm-patient-search-app/dist/807.js 1.12 kB 0 B
packages/esm-patient-search-app/dist/833.js 947 B 0 B
packages/esm-patient-search-app/dist/842.js 2.06 kB 0 B
packages/esm-patient-search-app/dist/939.js 4.42 kB 0 B
packages/esm-patient-search-app/dist/982.js 1.69 kB 0 B
packages/esm-patient-search-app/dist/main.js 3.44 kB 0 B
packages/esm-patient-search-app/dist/openmrs-esm-patient-search-app.js 3.19 kB 0 B

compressed-size-action

Copy link
Member

@vasharma05 vasharma05 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything else looks good!
Just doubtful that adding Layer is okay to add in here.
Thanks!

turbo.json Outdated Show resolved Hide resolved
@denniskigen denniskigen marked this pull request as draft July 25, 2023 21:18
@denniskigen denniskigen marked this pull request as ready for review July 27, 2023 20:16
@ibacher
Copy link
Member

ibacher commented Jul 27, 2023

This looks good to me, although, I probably look more later. I'm actually ok with having a jest.config.js for each project... just like we have a webpack.config.js, even though all that does is re-export a library. It may not be something we need now, but if an app develops the need for a specialised jest config, we'd already have the files.

@denniskigen
Copy link
Member Author

The patient chart PR revealed an issue with using a single jest config at the root of the project and referencing it in all the child packages' test script as the argument to --config - performance slows down to a literal crawl. So, using multiple configs as is the case with the latest commits is probably the best approach.

@denniskigen denniskigen merged commit 322db0b into main Jul 27, 2023
6 of 7 checks passed
@denniskigen denniskigen deleted the refactor/tests branch July 27, 2023 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants