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.

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

Fix empty coverage report #324

Merged
merged 3 commits into from
Jul 14, 2023
Merged

Fix empty coverage report #324

merged 3 commits into from
Jul 14, 2023

Conversation

bryanjtc
Copy link
Member

@bryanjtc bryanjtc commented Jun 28, 2023

Fixes: #313
This pull request introduces a code change aimed at resolving the issue of an empty coverage report in the project. The previous code checked whether the STORYBOOK_COLLECT_COVERAGE environment variable was not equal to 'true' before calling the reportCoverage() function. However, this approach did not cover all necessary conditions to ensure an accurate coverage report.

The new code includes an additional check to verify that both the STORYBOOK_COLLECT_COVERAGE environment variable is equal to 'true' and the JEST_SHARD environment variable is not equal to 'true'. This ensures that coverage reporting occurs only when both conditions are met, providing a more accurate assessment of test coverage.

By adding the process.env.JEST_SHARD !== 'true' condition, we prevent coverage reporting during Jest sharding, which helps prevent conflicts and inaccuracies in the coverage data.

📦 Published PR as canary version: 0.11.1--canary.324.e7f9b28.0

✨ Test out this PR locally via:

npm install @storybook/[email protected]
# or 
yarn add @storybook/[email protected]

Version

Published prerelease version: v0.11.1-next.0

Changelog

🐛 Bug Fix

Authors: 3

@bryanjtc bryanjtc requested a review from yannbf June 28, 2023 16:36
@bryanjtc bryanjtc self-assigned this Jun 28, 2023
@codecov
Copy link

codecov bot commented Jun 28, 2023

Codecov Report

Patch coverage has no change and project coverage change: +1.32 🎉

Comparison is base (2aa338d) 75.30% compared to head (e7f9b28) 76.63%.

Additional details and impacted files
@@            Coverage Diff             @@
##             next     #324      +/-   ##
==========================================
+ Coverage   75.30%   76.63%   +1.32%     
==========================================
  Files           8       11       +3     
  Lines         166      184      +18     
  Branches       35       39       +4     
==========================================
+ Hits          125      141      +16     
- Misses         41       43       +2     

see 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@yannbf
Copy link
Member

yannbf commented Jul 5, 2023

Thanks @bryanjtc ! Can you also check the following line

if (process.env.JEST_SHARD !== 'true') {

Move the condition for collecting coverage
to only allow it when the environment variable
STORYBOOK_COLLECT_COVERAGE is true and
JEST_SHARD is not true.
@bryanjtc
Copy link
Member Author

bryanjtc commented Jul 5, 2023

@yannbf Done

@yannbf
Copy link
Member

yannbf commented Jul 13, 2023

Hey @bryanjtc thanks! Thinking more about it, it seems like the logic is not correct.

The recipe you wrote tells people to:

mv coverage/storybook/coverage-storybook.json coverage/storybook/coverage-storybook-${matrix.shard}.json

And the logic in this PR doesn't run the coverage code when shard mode is on, so in the end you don't get the coverage/storybook/coverage-storybook.json file (but you do get the .nyc_output directory from jest-playwright). I fixed the logic so that:

  • coverage is always generated when --coverage is passed
  • coverage is only reported when --coverage is passed and --shard is NOT passed

let me know if that aligns with what you had in mind!

@bryanjtc
Copy link
Member Author

@yannbf yeah, it's ok

@yannbf yannbf added the patch Increment the patch version when merged label Jul 14, 2023
@yannbf yannbf merged commit 316d290 into next Jul 14, 2023
@bryanjtc bryanjtc deleted the fix-empty-coverage branch July 14, 2023 17:43
@yannbf yannbf mentioned this pull request Jul 27, 2023
@github-actions
Copy link

🚀 PR was released in v0.12.0 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch Increment the patch version when merged released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Coverage Report is Empty
2 participants