Add Visual Coverage Test + Combined Code Coverage for Coveralls #275

yen-tt · 2022-08-03T16:48:33Z

Use storybook test-runner and @storybook/addon-coverage to get visual coverage report. The setup requires spinning up storybook before generating the coverage numbers. Now, we have three test scripts:

test => combined test coverage
test:unit => jest test coverage only
test:visual => story book coverage only (note: this only collect coverage on src/components)

Updated coverage github action to also create/update comment which display code coverage percentage for unit, visual, and combined tests

the test-runner only works with jest v27 right now. As nidhi noted in her pr, storybook will be adding support for v28 shortly (storybookjs/test-runner#154).

playwright library is needed in the github action for test-storybook.

NOTE: The report generated from nyc merge command may not be super accurate after some comparison between to the unit vs visual vs merge report (ex: github issue).

J=SLAP-2269 & SLAP-2270
TEST=manual&auto

See that running npm run test:visual generates lcov report in coverage/visual folder. And the report is printed out in terminal.
See that a comment is made to the PR about the three coverage percentages.
See that coverall percentage increase without changes to tests (increased due to combined test coverage)

coveralls · 2022-08-03T16:51:07Z

Coverage increased (+0.7%) to 85.126% when pulling bddfa92 on dev/visual-coverage into cd9ea12 on main.

…y notices check

…have

… now. Add a pr comment for the visual coverage

juliannzhou · 2022-08-04T15:34:51Z

license/snyk issue has been resolved with legal approving use of dependency big-integer. Tom also said to disregard security/snyk issue because it is used for testing purposes only.

yen-tt · 2022-08-04T15:45:11Z

license/snyk issue has been resolved with legal approving use of dependency big-integer. Tom also said to disregard security/snyk issue because it is used for testing purposes only.

Will resolve these issues on synk site once this is merge to main.

oshi97 · 2022-08-04T15:49:24Z

Current unit coverage is 88.57374392220422% Current visual coverage is 78.16492450638792% Current combined coverage is 88.89789303079417%

Out of curiosity do you know why the get-coverage-percent coverage number is different than the coveralls number? I remember for regular coverage I was seeing a small percent difference and couldn't figure out exactly which numbers we wanted. It looks like the gap is a bit bigger now after including visual tests

yen-tt · 2022-08-04T15:53:59Z

get-coverage-percent coverage

I didn't look into the action get-coverage-percent coverage too much (versus how coveralls calculate things), so I'm not too sure. I can investigate for a bit to see if I find anything

oshi97 · 2022-08-04T15:54:48Z

I didn't look into the action get-coverage-percent coverage too much (versus how coveralls calculate things), so I'm not too sure. I can investigate for a bit to see if I find anything

if it's a little different it's fine for now just if you happened to know already

.github/workflows/coverage.yml

tests/scripts/visual-coverage.sh

tests/scripts/combined-coverage.sh

nmanu1 · 2022-08-04T16:38:10Z

license/snyk issue has been resolved with legal approving use of dependency big-integer. Tom also said to disregard security/snyk issue because it is used for testing purposes only.

in case you haven't already, can you add the newly approved license to our "Approved Licenses" spreadsheet?

.github/workflows/coverage.yml

tests/scripts/visual-coverage.sh

package.json

tests/scripts/visual-coverage.sh

nmanu1 · 2022-08-04T18:09:43Z

tests/scripts/combined-coverage.sh

+cp coverage/visual/coverage-storybook.json coverage/merge/coverage-storybook.json
+
+nyc report --reporter=lcov --reporter=text -t coverage/merge --report-dir coverage/merge
+cp coverage/merge/lcov.info coverage/lcov.info


to check my understanding, the coverage check will fail if the combined coverage drops, right? or is it just if the unit test coverage drops?

the coverage check uses the combined coverage file (I need to copy the combined lcov.info to top level because it's where the coverage github action expects it to be right now) so it would be when the combined coverage drops.

Use storybook test-runner and `@storybook/addon-coverage` to get visual coverage report. The setup requires spinning up storybook before generating the coverage numbers. Now, we have three test scripts: - test => combined test coverage - test:unit => jest test coverage only - test:visual => story book coverage only (note: this only collect coverage on src/components) Updated coverage github action to also create/update comment which display code coverage percentage for unit, visual, and combined tests the test-runner only works with jest v27 right now. As nidhi noted in her [pr](#274), storybook will be adding support for v28 shortly (storybookjs/test-runner#154). [playwright](https://www.npmjs.com/package/playwright) library is needed in the github action for test-storybook. <img width="600" alt="Screen Shot 2022-08-03 at 4 51 03 PM" src="https://user-images.githubusercontent.com/36055303/182709211-0189bad6-e978-4f82-9ee4-ba62be350283.png"> NOTE: The report generated from `nyc` merge command may not be super accurate after some comparison between to the unit vs visual vs merge report (ex: [github issue](istanbuljs/nyc#1302)). J=SLAP-2269 & SLAP-2270 TEST=manual&auto - See that running `npm run test:visual` generates lcov report in coverage/visual folder. And the report is printed out in terminal. - See that a comment is made to the PR about the three coverage percentages. - See that coverall percentage increase without changes to tests (increased due to combined test coverage)

Upgrade Jest from v27 to v29. We had [previously](#169) upgraded Jest to v28, but had to [downgrade](#275) to v27 because at the time, `@storybook/test-runner` only supported up to v27. In v0.7 of `@storybook/test-runner`, they changed Jest to be an internal dependency instead of a peer dependency, allowing us to use whatever version of Jest we want. There was only one breaking change in v29 that affected our code, besides those that already needed to be made when upgrading from v27 to v28 (see previous PR [description](#169 (comment))). This was the `jsdom` upgrade from v19 to v20 in `jest-environment-jsdom`, which requires the `typescript` version to be 4.5 or higher. With the new support added in v29 and `uuid` v9, we are now able to remove the manual resolving needed in `tests/__setup__/resolver.ts`. For more context on why this was initially needed with Jest v28, see #169. J=SLAP-2376 TEST=manual See that existing tests still run properly.

wip

ce0303f

bash script

e884ffc

yen-tt force-pushed the dev/visual-coverage branch from 7930772 to e884ffc Compare August 3, 2022 18:05

update merge

045154f

yen-tt force-pushed the dev/visual-coverage branch from aeb91ab to 045154f Compare August 3, 2022 18:47

github-actions bot and others added 10 commits August 3, 2022 18:49

Automated update to THIRD-PARTY-NOTICES from github action's 3rd part…

f21f56a

…y notices check

space

1c44b90

remove the merge command stuff, update run_test

fef4e58

use the udpated github action with coverage file param

7d5ade9

update test_script as well

594b07e

for base-coverage to work, use the same script name that main branch …

74d37a0

…have

update package json

8541580

use normal test script and lcov file in top level coverage folder for…

c420b8d

… now. Add a pr comment for the visual coverage

revert changes in run-test github action

07bc6d7

syntax fix

d359af3

yen-tt changed the title ~~wip~~ Add Visual Coverage Test Aug 3, 2022

Yen Truong added 8 commits August 3, 2022 16:18

add runs-on

9c01aad

more syntax

e5f47d5

run test:visual

b92ae6b

checkout repo

6f0bc4f

install

a6bef7f

github action test-storybook requires playwright

128fe2d

playwright install command

b1964a2

use npx instead

d88a26f

yen-tt mentioned this pull request Aug 3, 2022

add visual regression coverage #264

Closed

yen-tt marked this pull request as ready for review August 3, 2022 21:02

yen-tt requested a review from a team as a code owner August 3, 2022 21:02

Yen Truong added 2 commits August 3, 2022 17:03

cleanup

9fa48c8

curl instead of sleep

e40542c

only collect coverage for src/components for visual test

936fa7d

yext deleted a comment from github-actions bot Aug 4, 2022

yen-tt removed the wip label Aug 4, 2022

yen-tt changed the title ~~Add Visual Coverage Test~~ Add Visual Coverage Test + Combined Code Coverage for Coveralls Aug 4, 2022

update comments

2042fe6

oshi97 reviewed Aug 4, 2022

View reviewed changes

.github/workflows/coverage.yml Show resolved Hide resolved

tests/scripts/visual-coverage.sh Outdated Show resolved Hide resolved

tests/scripts/combined-coverage.sh Outdated Show resolved Hide resolved

nmanu1 reviewed Aug 4, 2022

View reviewed changes

.github/workflows/coverage.yml Show resolved Hide resolved

.github/workflows/coverage.yml Show resolved Hide resolved

reduce max attempts. remove merge-only flag

ed8f0bd

yen-tt requested review from oshi97 and nmanu1 August 4, 2022 17:23

tmeyer2115 previously approved these changes Aug 4, 2022

View reviewed changes

nmanu1 reviewed Aug 4, 2022

View reviewed changes

tests/scripts/visual-coverage.sh Outdated Show resolved Hide resolved

package.json Outdated Show resolved Hide resolved

tests/scripts/visual-coverage.sh Show resolved Hide resolved

update version, kill background job before terminate

e1082c6

yen-tt dismissed tmeyer2115’s stale review via e1082c6 August 4, 2022 18:03

nmanu1 reviewed Aug 4, 2022

View reviewed changes

remove test stuff

bddfa92

yen-tt requested review from nmanu1 and tmeyer2115 August 4, 2022 18:15

tmeyer2115 approved these changes Aug 4, 2022

View reviewed changes

yen-tt merged commit f3c4ad0 into main Aug 4, 2022

yen-tt deleted the dev/visual-coverage branch August 4, 2022 18:33

nmanu1 mentioned this pull request Sep 29, 2022

Upgrade Jest to v29 #309

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Visual Coverage Test + Combined Code Coverage for Coveralls #275

Add Visual Coverage Test + Combined Code Coverage for Coveralls #275

yen-tt commented Aug 3, 2022 •

edited

coveralls commented Aug 3, 2022 •

edited

juliannzhou commented Aug 4, 2022

yen-tt commented Aug 4, 2022

oshi97 commented Aug 4, 2022 •

edited

yen-tt commented Aug 4, 2022

oshi97 commented Aug 4, 2022

nmanu1 commented Aug 4, 2022

nmanu1 Aug 4, 2022

yen-tt Aug 4, 2022 •

edited

Add Visual Coverage Test + Combined Code Coverage for Coveralls #275

Add Visual Coverage Test + Combined Code Coverage for Coveralls #275

Conversation

yen-tt commented Aug 3, 2022 • edited

coveralls commented Aug 3, 2022 • edited

juliannzhou commented Aug 4, 2022

yen-tt commented Aug 4, 2022

oshi97 commented Aug 4, 2022 • edited

yen-tt commented Aug 4, 2022

oshi97 commented Aug 4, 2022

nmanu1 commented Aug 4, 2022

nmanu1 Aug 4, 2022

Choose a reason for hiding this comment

yen-tt Aug 4, 2022 • edited

Choose a reason for hiding this comment

yen-tt commented Aug 3, 2022 •

edited

coveralls commented Aug 3, 2022 •

edited

oshi97 commented Aug 4, 2022 •

edited

yen-tt Aug 4, 2022 •

edited