Skip to content

Conversation

@paulirish
Copy link
Member

over in #13125 i'm getting codecov failures and it appears our coverage isn't picking up tests in report, etc.

This appears to sort it out.

@paulirish paulirish requested a review from a team as a code owner October 20, 2021 21:43
@paulirish paulirish requested review from adamraine and removed request for a team October 20, 2021 21:43
@google-cla google-cla bot added the cla: yes label Oct 20, 2021
"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
Contributor

@brendankenny brendankenny Oct 20, 2021

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 .?

@brendankenny
Copy link
Contributor

Screen Shot 2021-10-20 at 17 03 37

codecov status continue to be a mystery though :)

@paulirish
Copy link
Member Author

(probably just a note for future us: when i just added (flow-)report into c8.sh, i only got coverage for report-generator. the report renderer stuff remained uncovered.. that was only fixed with the .. Again... don't really understand, but oh well.)


image

So that -0.41% doesn't look significant, but it is.

here's the "files" view of the two relevant commits on master:

image

359 files vs 400 files.

so yeah.. verified that these changes add back in coverage for report and flow-report.

:)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants