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

Addon Test: Fix printing null% for coverage #30061

Open
wants to merge 1 commit into
base: next
Choose a base branch
from

Conversation

ghengeveld
Copy link
Member

@ghengeveld ghengeveld commented Dec 13, 2024

What I did

Fixes this issue by not printing anything at all if the value is null or undefined:

image

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests
  • integration tests
  • end-to-end tests

Manual testing

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

Documentation

  • Add or update documentation reflecting your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Checklist for Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli-storybook/src/sandbox-templates.ts

  • Make sure this PR contains one of the labels below:

    Available labels
    • bug: Internal changes that fixes incorrect behavior.
    • maintenance: User-facing maintenance tasks.
    • dependencies: Upgrading (sometimes downgrading) dependencies.
    • build: Internal-facing build tooling & test updates. Will not show up in release changelog.
    • cleanup: Minor cleanup style change. Will not show up in release changelog.
    • documentation: Documentation only changes. Will not show up in release changelog.
    • feature request: Introducing a new feature.
    • BREAKING CHANGE: Changes that break compatibility in some way with current major version.
    • other: Changes that don't fit in the above categories.

🦋 Canary release

This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>

name before after diff z %
createSize 0 B 0 B 0 B - -
generateSize 77.7 MB 77.7 MB 0 B 0.42 0%
initSize 133 MB 133 MB 0 B 0.72 0%
diffSize 55.3 MB 55.3 MB 0 B 0.72 0%
buildSize 7.24 MB 7.24 MB 0 B 4.36 0%
buildSbAddonsSize 1.88 MB 1.88 MB 0 B 4.36 0%
buildSbCommonSize 195 kB 195 kB 0 B - 0%
buildSbManagerSize 1.86 MB 1.86 MB 0 B 3.03 0%
buildSbPreviewSize 0 B 0 B 0 B - -
buildStaticSize 0 B 0 B 0 B - -
buildPrebuildSize 3.93 MB 3.93 MB 0 B 4.36 0%
buildPreviewSize 3.3 MB 3.3 MB 0 B 3.74 0%
testBuildSize 0 B 0 B 0 B - -
testBuildSbAddonsSize 0 B 0 B 0 B - -
testBuildSbCommonSize 0 B 0 B 0 B - -
testBuildSbManagerSize 0 B 0 B 0 B - -
testBuildSbPreviewSize 0 B 0 B 0 B - -
testBuildStaticSize 0 B 0 B 0 B - -
testBuildPrebuildSize 0 B 0 B 0 B - -
testBuildPreviewSize 0 B 0 B 0 B - -
name before after diff z %
createTime 26.7s 7.7s -19s -6ms -0.73 -244.9%
generateTime 23.2s 21.7s -1s -455ms -0.19 -6.7%
initTime 14.9s 14.9s 26ms -0.18 0.2%
buildTime 9.1s 8.6s -409ms -0.52 -4.7%
testBuildTime 0ms 0ms 0ms - -
devPreviewResponsive 6s 4.2s -1s -864ms -1.45 🔰-44%
devManagerResponsive 4.5s 3.3s -1s -241ms -1.23 -37.6%
devManagerHeaderVisible 606ms 511ms -95ms -0.74 -18.6%
devManagerIndexVisible 637ms 538ms -99ms -1.17 -18.4%
devStoryVisibleUncached 1.9s 1.7s -245ms -0.37 -14.4%
devStoryVisible 635ms 539ms -96ms -1.09 -17.8%
devAutodocsVisible 459ms 455ms -4ms -0.77 -0.9%
devMDXVisible 636ms 472ms -164ms -0.52 -34.7%
buildManagerHeaderVisible 704ms 490ms -214ms -1 -43.7%
buildManagerIndexVisible 806ms 577ms -229ms -1.04 -39.7%
buildStoryVisible 658ms 455ms -203ms -0.91 -44.6%
buildAutodocsVisible 540ms 412ms -128ms -0.51 -31.1%
buildMDXVisible 581ms 389ms -192ms -0.72 -49.4%

Greptile Summary

This PR fixes an issue in the test addon where coverage percentages were incorrectly displaying "null%" when no coverage data was available.

  • Modified code/addons/test/src/components/TestProviderRender.tsx to prevent displaying percentage when value is null/undefined
  • Potential logical error in fix implementation with nullish coalescing operator that could still display "null%"
  • Missing test coverage to verify the fix works as intended
  • Affects coverage reporting display in Storybook's test addon UI

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

1 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile

@@ -314,7 +314,7 @@ export const TestProviderRender: FC<
aria-label={`status: ${coverageSummary.status}`}
/>
}
right={`${coverageSummary.percentage}%`}
right={coverageSummary.percentage ?? `${coverageSummary.percentage}%`}
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: This line has a logical error - when coverageSummary.percentage is null, it will display 'null%' because of incorrect use of the nullish coalescing operator. Should be coverageSummary.percentage ? ${coverageSummary.percentage}% : null

Copy link

nx-cloud bot commented Dec 13, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit f0f4e48. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

@storybook-pr-benchmarking
Copy link

storybook-pr-benchmarking bot commented Dec 13, 2024

Package Benchmarks

Commit: f0f4e48, ran on 13 December 2024 at 15:56:54 UTC

The following packages have significant changes to their size or dependencies:

@storybook/builder-webpack5

Before After Difference
Dependency count 230 234 🚨 +4 🚨
Self size 79 KB 79 KB 0 B
Dependency size 29.77 MB 31.03 MB 🚨 +1.26 MB 🚨
Bundle Size Analyzer Link Link

@storybook/angular

Before After Difference
Dependency count 265 261 🎉 -4 🎉
Self size 363 KB 363 KB 0 B
Dependency size 34.67 MB 33.56 MB 🎉 -1.11 MB 🎉
Bundle Size Analyzer Link Link

@storybook/ember

Before After Difference
Dependency count 257 253 🎉 -4 🎉
Self size 22 KB 22 KB 0 B
Dependency size 32.17 MB 31.06 MB 🎉 -1.11 MB 🎉
Bundle Size Analyzer Link Link

@storybook/html-webpack5

Before After Difference
Dependency count 240 244 🚨 +4 🚨
Self size 6 KB 6 KB 0 B
Dependency size 30.32 MB 31.58 MB 🚨 +1.26 MB 🚨
Bundle Size Analyzer Link Link

@storybook/nextjs

Before After Difference
Dependency count 593 589 🎉 -4 🎉
Self size 471 KB 471 KB 0 B
Dependency size 84.29 MB 83.19 MB 🎉 -1.11 MB 🎉
Bundle Size Analyzer Link Link

@storybook/preact-webpack5

Before After Difference
Dependency count 238 242 🚨 +4 🚨
Self size 6 KB 6 KB 0 B
Dependency size 29.89 MB 31.16 MB 🚨 +1.26 MB 🚨
Bundle Size Analyzer Link Link

@storybook/react-webpack5

Before After Difference
Dependency count 316 320 🚨 +4 🚨
Self size 6 KB 6 KB 0 B
Dependency size 41.06 MB 42.32 MB 🚨 +1.26 MB 🚨
Bundle Size Analyzer Link Link

@storybook/server-webpack5

Before After Difference
Dependency count 248 252 🚨 +4 🚨
Self size 14 KB 14 KB 0 B
Dependency size 31.30 MB 32.56 MB 🚨 +1.26 MB 🚨
Bundle Size Analyzer Link Link

@storybook/svelte-webpack5

Before After Difference
Dependency count 303 307 🚨 +4 🚨
Self size 6 KB 6 KB 0 B
Dependency size 37.89 MB 39.15 MB 🚨 +1.26 MB 🚨
Bundle Size Analyzer Link Link

@storybook/vue3-webpack5

Before After Difference
Dependency count 489 493 🚨 +4 🚨
Self size 6 KB 6 KB 0 B
Dependency size 54.29 MB 55.55 MB 🚨 +1.26 MB 🚨
Bundle Size Analyzer Link Link

@storybook/web-components-webpack5

Before After Difference
Dependency count 238 242 🚨 +4 🚨
Self size 5 KB 5 KB 0 B
Dependency size 29.94 MB 31.20 MB 🚨 +1.26 MB 🚨
Bundle Size Analyzer Link Link

@storybook/preset-html-webpack

Before After Difference
Dependency count 93 102 🚨 +9 🚨
Self size 4 KB 4 KB 0 B
Dependency size 20.23 MB 22.71 MB 🚨 +2.48 MB 🚨
Bundle Size Analyzer Link Link

@storybook/preset-react-webpack

Before After Difference
Dependency count 182 191 🚨 +9 🚨
Self size 24 KB 24 KB 0 B
Dependency size 30.99 MB 33.47 MB 🚨 +2.48 MB 🚨
Bundle Size Analyzer Link Link

@storybook/preset-vue3-webpack

Before After Difference
Dependency count 369 378 🚨 +9 🚨
Self size 9 KB 9 KB 0 B
Dependency size 45.16 MB 47.64 MB 🚨 +2.48 MB 🚨
Bundle Size Analyzer Link Link

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.

1 participant