Track test coverage of JS packages in Coveralls #20911

diedexx · 2023-12-05T09:22:23Z

Context

We want to make the impact a PR makes on our test coverage clear to everyone. We want to see if a change has an unexpected increase or decrease in test coverage and we want to have extra motivation/an extra reminder to increase coverage.

Summary

This PR can be summarized in the following changelog entry:

Upload test coverage of all our tested JavaScript packages to Coveralls.io
Separate Jest test configuration from the package.json.

Relevant technical choices:

I specify collectCoverageFrom for all packages. Without this setting, Jest will only calculate coverage over tested files. We don't see if there is a file that has no tests covering them.
I changed the default test behaviour for three packages, so they're consistent with the others:
1. packages/js (diff): With -u, we simply accept the result of any change we make that changes the outcome of any of our snapshot tests. To spot issues easly, let the tests fail on failing snapshot tests, instead of noticing changes to snapshot files when you are ready to commit your changes.
2. packages/feature-flag (diff): I removed --coverage, simply for consistency with other packages. In theory the tests are a bit faster, but I didn't measure a meaningful difference.
3. packages/yoastseo(diff): I change the default jest config to not collect coverage by default for the same reasons as above.
I add all flags for all tests we do in the carryforward list. We only run JS tests if there are changes impacting JS, and the same applies to PHP. Because of that, we might not have coverage for the entire repo, skewing the coverage reporting. So, wo carry previous run results forward if those tests haven't been executed.

Test instructions

Test instructions for the acceptance test before the PR gets merged

This PR can be acceptance tested by following these steps:

Check the CI runs for this PR (pull request).
- Check the latest Coveralls build: Build 7099535423. All tested projects should be there and the coverage should be accurate.
Create a new feature branch and push the HEAD of this PR to it. That should trigger a test run (push).
- When the test run is done, a new build should be available in Coveralls.io.

Relevant test scenarios

Changes should be tested with the browser console open
Changes should be tested on different posts/pages/taxonomies/custom post types/custom taxonomies
Changes should be tested on different editors (Block/Classic/Elementor/other)
Changes should be tested on different browsers
Changes should be tested on multisite

Test instructions for QA when the code is in the RC

[n/a] QA should use the same steps as above.

Impact check

This PR affects the following parts of the plugin, which may require extra testing:

The yarn test script for yoastseo, feature-flag and js.

UI changes

This PR changes the UI in the plugin. I have added the 'UI change' label to this PR.

Other environments

This PR also affects Shopify. I have added a changelog entry starting with [shopify-seo], added test instructions for Shopify and attached the Shopify label to this PR.

Documentation

I have written documentation for this change.

Quality assurance

I have tested this code to the best of my abilities.
During testing, I had activated all plugins that Yoast SEO provides integrations for.
I have added unit tests to verify the code works as intended.
If any part of the code is behind a feature flag, my test instructions also cover cases where the feature flag is switched off.
I have written this PR in accordance with my team's definition of done.
I have checked that the base branch is correctly set.

Innovation

No innovation project is applicable for this PR.
This PR falls under an innovation project. I have attached the innovation label.
I have added my hours to the WBSO document.

Take the Jest configuration out of the package json and into its own separate config file, where we export a JS object instead of having to use JSON. This allows us slightly more flexibility. We could add comments for example or use variables.

yarn test --coverage will generate json, lcov, text and clover by default. I only add text-summary to the list.

With -u, we simply accept the result of any change we make that changes the outcome of any of our snapshot tests. To spot issues easly, let the tests fail on failing snapshot tests, instead of noticing changes to snapshot files when you are ready to commit your changes.

This is consistent with all our other packages and makes for slightly faster test-runs.

coveralls · 2023-12-05T09:54:18Z

Pull Request Test Coverage Report for Build 7272587514

Warning: This coverage report may be inaccurate.

We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
To ensure accuracy in future PRs, please see these guidelines.
A quick fix for this PR: rebase it; your next report should be accurate.

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+3.7%) to 53.1%

Totals
Change from base Build 7262536377:	3.7%
Covered Lines:	28900
Relevant Lines:	54818

💛 - Coveralls

igorschoester

Awesome work! 🥳

Mainly some comments about further polishing of the configurations.
It seems that the moduleNameMapper needs lodash-es if yoastseo is involved. And .css is needed for @yoast/components at least.
I'm not following why the transforms are not needed anymore when yoastseo is still involved.

packages/components/jest.config.js

packages/feature-flag/jest.config.js

packages/helpers/jest.config.js

packages/replacement-variable-editor/jest.config.js

igorschoester · 2023-12-06T11:52:30Z

packages/search-metadata-previews/package.json

-    "transformIgnorePatterns": [
-      "/node_modules/(?!yoastseo|lodash-es).+\\.js$"
-    ],


Interesting, I would think the yoastseo exception is still needed here, and therefor also the lodash-es. But it works anyway with just the lodash-es map you have kept 🤔

Do you have any insights on this?

Very interesting indeed. I wasn't able figure out what exactly is going on, but it seems like babel-jest is picking up babelrc files from its linked dependencies regardless of transformIgnorePatterns. Or perhaps it being a link, makes the ignorePattern not match

packages/social-metadata-forms/jest.config.js

packages/social-metadata-previews/jest.config.js

.github/workflows/finish-coveralls.yml

Co-authored-by: Igor <[email protected]>

github-actions · 2023-12-19T10:10:26Z

@diedexx Please be aware that following packages have been abandoned and are not actively maintained anymore:

Package name	Path
yoast-components	packages/yoast-components
@yoast/babel-preset	packages/babel-preset
@yoast/e2e-tests	packages/e2e-tests
@yoast/helpers	packages/helpers
@yoast/jest-preset	packages/jest-preset
@yoast/style-guide	packages/style-guide

Please consider using the other packages instead.

diedexx added 5 commits December 1, 2023 16:49

Ensure consistency in Jest config

ddeeace

Take the Jest configuration out of the package json and into its own separate config file, where we export a JS object instead of having to use JSON. This allows us slightly more flexibility. We could add comments for example or use variables.

Generate coverage reports over all source files, even untested ones

53d9558

yarn test --coverage will generate json, lcov, text and clover by default. I only add text-summary to the list.

Don't collect coverage by default

b166963

This is consistent with all our other packages and makes for slightly faster test-runs.

Send JS packages' coverage to coveralls

ca1bfe8

diedexx force-pushed the coveralls branch from dca0ae7 to ca1bfe8 Compare December 5, 2023 09:44

diedexx force-pushed the coveralls branch 2 times, most recently from 313514e to 41f95d4 Compare December 5, 2023 10:28

Don't run if none of the tests are running

d39d63d

diedexx force-pushed the coveralls branch from 41f95d4 to d39d63d Compare December 5, 2023 10:38

diedexx added the changelog: non-user-facing Needs to be included in the 'Non-userfacing' category in the changelog label Dec 5, 2023

diedexx modified the milestone: 21.8 Dec 5, 2023

diedexx marked this pull request as ready for review December 5, 2023 15:16

diedexx requested a review from a team as a code owner December 5, 2023 15:16

noud-github self-assigned this Dec 6, 2023

igorschoester reviewed Dec 6, 2023

View reviewed changes

Apply suggestions from code review

3611e2e

Co-authored-by: Igor <[email protected]>

Base automatically changed from feature/upgrade-react-and-tests to trunk December 13, 2023 08:58

Merge branch 'trunk' of github.com:Yoast/wordpress-seo into coveralls

befd613

diedexx added 2 commits December 19, 2023 17:37

Merge branch 'trunk' of github.com:Yoast/wordpress-seo into coveralls

723dc8b

Update WP versions in test flags

170c84a

diedexx merged commit 99b0d9f into trunk Dec 20, 2023
30 checks passed

diedexx deleted the coveralls branch December 20, 2023 09:11

mhkuu added this to the 21.9 milestone Dec 28, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Track test coverage of JS packages in Coveralls #20911

Track test coverage of JS packages in Coveralls #20911

diedexx commented Dec 5, 2023 •

edited

Loading

coveralls commented Dec 5, 2023 •

edited

Loading

igorschoester left a comment

igorschoester Dec 6, 2023

diedexx Dec 8, 2023 •

edited

Loading

github-actions bot commented Dec 19, 2023

Track test coverage of JS packages in Coveralls #20911

Track test coverage of JS packages in Coveralls #20911

Conversation

diedexx commented Dec 5, 2023 • edited Loading

Context

Summary

Relevant technical choices:

Test instructions

Test instructions for the acceptance test before the PR gets merged

Relevant test scenarios

Test instructions for QA when the code is in the RC

Impact check

UI changes

Other environments

Documentation

Quality assurance

Innovation

coveralls commented Dec 5, 2023 • edited Loading

Pull Request Test Coverage Report for Build 7272587514

Warning: This coverage report may be inaccurate.

💛 - Coveralls

igorschoester left a comment

Choose a reason for hiding this comment

igorschoester Dec 6, 2023

Choose a reason for hiding this comment

diedexx Dec 8, 2023 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Dec 19, 2023

diedexx commented Dec 5, 2023 •

edited

Loading

coveralls commented Dec 5, 2023 •

edited

Loading

diedexx Dec 8, 2023 •

edited

Loading