-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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: run code coverage in github actions #11770
Conversation
Tom from Codecov here. I'm not sure why statuses haven't come up, but if you push a new commit here, I can take a look! |
At first glance, looks like this is the problem. My guess is we do not properly handle the emojis (💡🏠) as workflow/job names. I'll open up a ticket with the product team about this, and I'll work on a fix in the uploader to deploy this week or next. |
Great find, thanks for the help! Ahh, vindication...can we remove these emojis now folks? :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really excited about this! Can we land? Seems like we could figure out the emoji thing afterwards
@@ -27,7 +27,6 @@ | |||
* promise. Instead, map to a successful object that contains this information. | |||
* @param {string|Error} err The error to convert | |||
*/ | |||
/* istanbul ignore next */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe worth adding to the fileoverview that this whole file is ignored for code coverage since it's intended to run in the browser?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that information useful to someone looking/editing/using functions form this module? Seems a concern only if you are looking at how c8 works, and naturally you'd be looking at c8.sh.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that information useful to someone looking/editing/using functions form this module?
I think so. There's coverage magic going on and the only way to figure out why e.g. a new page function in here doesn't have to be ignored while a new page function that's defined within a gatherer file does have to be ignored is to know you have to look in c8.sh
in the first place
"unit-cli": "jest \"lighthouse-cli/\"", | ||
"unit-viewer": "jest \"lighthouse-viewer/.*-test.js\"", | ||
"unit": "yarn unit-core && yarn unit-cli && yarn unit-viewer", | ||
"unit:ci": "npm run unit-core -- --runInBand --ci --coverage && npm run unit-cli -- --runInBand --ci --coverage && npm run unit-viewer -- --runInBand --ci --coverage", | ||
"unit:ci": "NODE_OPTIONS=--max-old-space-size=4096 npm run unit-core -- --ci && npm run unit-cli -- --ci && npm run unit-viewer -- --ci", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NODE_OPTIONS=--max-old-space-size=4096
is this still necessary? Seems like you added it originally for fixing jest coverage, not c8 coverage?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup, just saw it fails without in ubuntu
yarn.lock
Outdated
"@types/istanbul-lib-coverage@*", "@types/istanbul-lib-coverage@^2.0.0": | ||
version "2.0.1" | ||
resolved "https://registry.yarnpkg.com/@types/istanbul-lib-coverage/-/istanbul-lib-coverage-2.0.1.tgz#42995b446db9a48a11a07ec083499a860e9138ff" | ||
integrity sha512-hRJD2ahnnpLgsj6KWMYSrmXkM3rm2Dl1qkx6IOFD5FnuNPXJIG5L0dhgKXCYTRMGzU4n0wImQ/xfmRc4POUFlg== | ||
|
||
"@types/istanbul-lib-coverage@^2.0.1": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
relatively minor, but if you want to eliminate some unneeded duplicate new deps:
diff --git a/yarn.lock b/yarn.lock
index 96b73a0bd..6b74d3fb5 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -566,12 +566,7 @@
resolved "https://registry.yarnpkg.com/@types/is-windows/-/is-windows-1.0.0.tgz#1011fa129d87091e2f6faf9042d6704cdf2e7be0"
integrity sha512-tJ1rq04tGKuIJoWIH0Gyuwv4RQ3+tIu7wQrC0MV47raQ44kIzXSSFKfrxFUOWVRvesoF7mrTqigXmqoZJsXwTg==
-"@types/istanbul-lib-coverage@*", "@types/istanbul-lib-coverage@^2.0.0":
- version "2.0.1"
- resolved "https://registry.yarnpkg.com/@types/istanbul-lib-coverage/-/istanbul-lib-coverage-2.0.1.tgz#42995b446db9a48a11a07ec083499a860e9138ff"
- integrity sha512-hRJD2ahnnpLgsj6KWMYSrmXkM3rm2Dl1qkx6IOFD5FnuNPXJIG5L0dhgKXCYTRMGzU4n0wImQ/xfmRc4POUFlg==
-
-"@types/istanbul-lib-coverage@^2.0.1":
+"@types/istanbul-lib-coverage@*", "@types/istanbul-lib-coverage@^2.0.0", "@types/istanbul-lib-coverage@^2.0.1":
version "2.0.3"
resolved "https://registry.yarnpkg.com/@types/istanbul-lib-coverage/-/istanbul-lib-coverage-2.0.3.tgz#4ba8ddb720221f432e443bd5f9117fd22cfd4762"
integrity sha512-sz7iLqvVUg1gIedBOvlkxPlc8/uVzyS5OwGz1cKjXzkl3FpL3al0crU8YGU1WoHkxn0Wxbw5tyi6hvzJKNzFsw==
@@ -3501,19 +3496,7 @@ glob-to-regexp@^0.3.0:
resolved "https://registry.yarnpkg.com/glob-to-regexp/-/glob-to-regexp-0.3.0.tgz#8c5a1494d2066c570cc3bfe4496175acc4d502ab"
integrity sha1-jFoUlNIGbFcMw7/kSWF1rMTVAqs=
-glob@^7.0.0, glob@^7.0.3, glob@^7.1.0, glob@^7.1.1, glob@^7.1.2, glob@^7.1.3:
- version "7.1.4"
- resolved "https://registry.yarnpkg.com/glob/-/glob-7.1.4.tgz#aa608a2f6c577ad357e1ae5a5c26d9a8d1969255"
- integrity sha512-hkLPepehmnKk41pUGm3sYxoFs/umurYfYJCerbXEyFIWcAzvpipAgVkBqqT9RBKMGjnq6kMuyYwha6csxbiM1A==
- dependencies:
- fs.realpath "^1.0.0"
- inflight "^1.0.4"
- inherits "2"
- minimatch "^3.0.4"
- once "^1.3.0"
- path-is-absolute "^1.0.0"
-
-glob@^7.1.4:
+glob@^7.0.0, glob@^7.0.3, glob@^7.1.0, glob@^7.1.1, glob@^7.1.2, glob@^7.1.3, glob@^7.1.4:
version "7.1.6"
resolved "https://registry.yarnpkg.com/glob/-/glob-7.1.6.tgz#141f33b81a7c2492e125594307480c46679278a6"
integrity sha512-LwaxwyZ72Lk7vZINtNNrywX0ZuLyStrdDtabefZKAY5ZGJhVtgdznluResxNmPitE0SAO+O26sWTHeKSI2wMBA==
@@ -8328,7 +8311,7 @@ yargs@^15.0.0, yargs@^15.3.1:
y18n "^4.0.0"
yargs-parser "^18.1.2"
-yargs@^16.0.0:
+yargs@^16.0.0, yargs@^16.1.1:
version "16.2.0"
resolved "https://registry.yarnpkg.com/yargs/-/yargs-16.2.0.tgz#1c82bf0f6b6a66eafce7ef30e376f49a12477f66"
integrity sha512-D1mvvtDG0L5ft/jGWkLpG1+m0eQxOfaBvTNELraWj22wSVUMWxZUvYgJYcKh6jGGIkJFhH4IZPQhR4TKpc8mBw==
@@ -8341,19 +8324,6 @@ yargs@^16.0.0:
y18n "^5.0.5"
yargs-parser "^20.2.2"
-yargs@^16.1.1:
- version "16.1.1"
- resolved "https://registry.yarnpkg.com/yargs/-/yargs-16.1.1.tgz#5a4a095bd1ca806b0a50d0c03611d38034d219a1"
- integrity sha512-hAD1RcFP/wfgfxgMVswPE+z3tlPFtxG8/yWUrG2i17sTWGCGqWnxKcLTF4cUKDUK8fzokwsmO9H0TDkRbMHy8w==
- dependencies:
- cliui "^7.0.2"
- escalade "^3.1.1"
- get-caller-file "^2.0.5"
- require-directory "^2.1.1"
- string-width "^4.2.0"
- y18n "^5.0.5"
- yargs-parser "^20.2.2"
-
yauzl@^2.10.0:
version "2.10.0"
resolved "https://registry.yarnpkg.com/yauzl/-/yauzl-2.10.0.tgz#c7eb17c93e112cb1086fa6d8e51fb0667b79a5f9"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there an automated process to do what you're doing manually?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there an automated process to do what you're doing manually?
I haven't seen any good tools that will de-dupe but try to minimize the churn but I haven't looked in a while
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops, meant to approve :)
The issue should be resolved on the Codecov side now @connorjclark |
This also replaces istanbul with c8, as coverage using istanbul has started to crash node.
part of #11194
fixes #11781