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.

Sign up for GitHub

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: ci tests should include all files #13235

Merged
merged 4 commits into from
Oct 21, 2021
Merged

tests: ci tests should include all files #13235

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
aaa27ba
tests: ci tests should include all files
paulirish Oct 20, 2021
aa79142
driveby: include (flow-)report in coverage
paulirish Oct 20, 2021
904f408
no temp dir
paulirish Oct 20, 2021
7a16265
Merge branch 'master' into testjestci
brendankenny Oct 20, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lighthouse-core/scripts/c8.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ set -euxo pipefail
echo $*

node node_modules/.bin/c8 \
--include '{lighthouse-core,lighthouse-cli,lighthouse-viewer,lighthouse-treemap}' \
--include '{lighthouse-core,lighthouse-cli,lighthouse-viewer,lighthouse-treemap,report,flow-report}' \
--exclude third_party \
--exclude '**/test/' \
--exclude '**/scripts/' \
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
"unit-viewer": "yarn jest \"lighthouse-viewer/.*-test.js\"",
"unit-flow": "yarn jest \"flow-report/.*-test.[tj]s[x]?\"",
"unit": "yarn jest",
"unit:ci": "NODE_OPTIONS=--max-old-space-size=8192 npm run jest -- --ci",
"unit:ci": "NODE_OPTIONS=--max-old-space-size=8192 npm run jest --ci .",
Copy link
Member Author

Choose a reason for hiding this comment

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

i SO wish this dot didn't have any material effect, but it appears to.

Copy link
Member

@brendankenny brendankenny Oct 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

whoops, that's from #13148. Interesting that you need the extra -- for npm run [script] but not for npm run [bin-thing]

well...it makes a certain amount of sense but it shouldn't have to :)

Copy link
Member Author

Choose a reason for hiding this comment

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

whoops, that's from #13148. Interesting that you need the extra -- for npm run [script] but not for npm run [bin-thing]

well...it makes a certain amount of sense but it shouldn't have to :)

to be completely honest i didn't verify the -- change is good. (or that --ci still works as expected.) it seems possible i regressed this.

i know yarn XX doesnt need extra --, but don't know npm run as well.

@brendankenny do you think we should change this back? also.. to what? npm run jest -- --ci .?

"core-unit": "yarn unit-core",
"cli-unit": "yarn unit-cli",
"viewer-unit": "yarn unit-viewer",
Expand Down