Skip to content

Conversation

@jmcreasman
Copy link
Contributor

@jmcreasman jmcreasman commented Jul 23, 2025

Per this Chapter task we're adopting a tool called Codecov in order to see code coverage across our repos. With Codecov it will compare branches with main and make sure we maintain certain percentage thresholds in test coverage. We'll be able to see which parts of the code need improvement and prevent too much uncovered code from making it into main. Per this RFC this tool has now been approved. We're starting with MC App Kit and have everything configured already in this branch and its reporting to Codecov. However, in order to see the complete suite of data display options in the Codecov Dashboard we need this code in main.

Percentage thresholds have not yet been added so merging this PR will not prevent other PRs from being merged yet due to low coverage. This is just to complete a POC so we can see all the features Codecov offers and make our decision about how to roll out to the other repos.

@jmcreasman jmcreasman self-assigned this Jul 23, 2025
@jmcreasman jmcreasman requested a review from a team as a code owner July 23, 2025 20:12
@jmcreasman jmcreasman added the fe-chapter-rotation Tasks coming from frontend chapter work label Jul 23, 2025
@changeset-bot
Copy link

changeset-bot bot commented Jul 23, 2025

⚠️ No Changeset found

Latest commit: 2dddaf9

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Jul 23, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
mc-app-kit-playground Ready Ready Preview Comment Nov 14, 2025 5:04pm
merchant-center-application-kit-components-playground Ready Ready Preview Comment Nov 14, 2025 5:04pm

@codecov-commenter
Copy link

codecov-commenter commented Jul 23, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@da206d9). Learn more about missing BASE report.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #3815   +/-   ##
=======================================
  Coverage        ?   76.10%           
=======================================
  Files           ?      272           
  Lines           ?     7115           
  Branches        ?     2305           
=======================================
  Hits            ?     5415           
  Misses          ?     1674           
  Partials        ?       26           
Components Coverage Δ
Application Components 81.18% <0.00%> (?)
Application Shell 77.09% <0.00%> (?)
Application Shell Connectors 77.68% <0.00%> (?)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update da206d9...2dddaf9. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jmcreasman jmcreasman marked this pull request as draft July 24, 2025 01:12
@jmcreasman jmcreasman changed the title Codecov POC Codecov Coverage Reports Aug 6, 2025
@jmcreasman jmcreasman changed the title Codecov Coverage Reports FEC-61 Codecov Coverage Reports Aug 6, 2025
@jmcreasman jmcreasman removed the request for review from a team August 6, 2025 14:39
@jmcreasman jmcreasman marked this pull request as ready for review August 6, 2025 15:04
@jmcreasman jmcreasman requested a review from a team August 6, 2025 15:04
Copy link
Contributor

@ByronDWall ByronDWall left a comment

Choose a reason for hiding this comment

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

I think that tracking code coverage is a good addition, and this is a good start at implementation. I also have a few concerns which I think merit some discussion before we move forward

@valoriecarli
Copy link
Contributor

11/12/2025 - per Carlos:
Please keep this PR open
We're waiting for someone to continue the work over there.

Comment on lines +206 to +217
exclude: [
'**/*.spec.js',
'**/*.spec.tsx',
'**/*.spec.ts',
'**/*.spec.jsx',
'**/*.test.js',
'**/*.test.tsx',
'**/*.test.ts',
'**/*.test.jsx',
'**/node_modules/**',
'**/test-utils/**',
],
Copy link
Contributor

Choose a reason for hiding this comment

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

Excludes test files from Istanbul coverage instrumentation. Coverage reports should only measure application code, not test code.

Comment on lines +87 to +108
// Workaround for Radix UI CSS selector parsing issues in JSDOM
// Radix UI generates invalid CSS selectors that cause JSDOM to throw errors
if (typeof window !== 'undefined') {
const originalGetComputedStyle = window.getComputedStyle;
window.getComputedStyle = function (element, pseudoElement) {
try {
return originalGetComputedStyle.call(this, element, pseudoElement);
} catch (error) {
// Return a mock CSSStyleDeclaration for CSS parsing errors
// This prevents tests from crashing due to invalid Radix UI selectors
if (error.message && error.message.includes('not a valid selector')) {
return {
getPropertyValue: () => '',
length: 0,
item: () => null,
[Symbol.iterator]: function* () {},
};
}
throw error;
}
};
}
Copy link
Contributor

@kterry1 kterry1 Nov 14, 2025

Choose a reason for hiding this comment

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

The workaround doesn't remove selectors - it catches CSS parsing errors that occur when JSDOM's CSS parser (nwsapi) encounters the malformed selectors that Radix UI generates.

What the workaround does:
The window.getComputedStyle workaround in setup-test-framework.js:

  • Wraps the original function in a try-catch block
  • Catches CSS parsing errors when JSDOM tries to parse invalid selectors like 'h2#radix-,,,Ara,,,A :focus-visible'
  • Returns a mock CSSStyleDeclaration object instead of throwing an error
  • Only affects test environments (JSDOM), not production

What it doesn't do:

  • It doesn't remove or modify the selectors themselves
  • It doesn't prevent Radix UI from generating the malformed selectors
  • It doesn't affect how the components render in tests
  • It doesn't affect production behavior at all

@kterry1 kterry1 self-requested a review November 14, 2025 17:12
Copy link
Contributor

@kterry1 kterry1 left a comment

Choose a reason for hiding this comment

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

Approving as I pushed up 2 changes that got the PR passing but haven't looked into how to view the dashboard for the overall PR.

@tdeekens
Copy link
Contributor

Screenshot 2025-11-17 at 10 16 19@2x

I think they were pushed we just miss the main base having anything.

@kterry1 kterry1 merged commit a23bb5e into main Nov 17, 2025
21 checks passed
@kterry1 kterry1 deleted the FEC-61-codecov branch November 17, 2025 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fe-chapter-rotation Tasks coming from frontend chapter work

Projects

None yet

Development

Successfully merging this pull request may close these issues.