Skip to content

Change the tests that check co2 values so they don't fail on every co2 dependency update - #5475

Merged
canova merged 2 commits into
firefox-devtools:mainfrom
canova:co2-test-fix
May 28, 2025
Merged

Change the tests that check co2 values so they don't fail on every co2 dependency update#5475
canova merged 2 commits into
firefox-devtools:mainfrom
canova:co2-test-fix

Conversation

@canova

@canovacanova commented May 28, 2025

Copy link
Copy Markdown
Member

I wanted to do that for a while but finally got around to it. The latest PR that we had to update the expectations was #5474. It's not a huge thing, but it's distracting enough that triggers a context switch. So this PR changes there parts to make sure that the tests don't fail on every co2 package update

I replaced the snapshot test with a more granular test. Technically there were ways to keep the snapshot tests by doing some hacky things, but I didn't want to do them. See the commit message for more info.

canova added 2 commits May 28, 2025 12:38
Previously we had a specific value for the expected co2 number, which
was problematic because whenever we had to upgrade the co2 package, we
had to update these values too. It's a small annoyance, but it can be
still annoying, especially you had to stop what you were doing and just
update the values of the tests.
So instead now we expect any values for these tests expectations. That
way we can upgrade the co2 package without problem!
Similar reason to the previous commit, but we had to change this
snapshot test into a more granular test that checks individual fields
because it's not very easy to get the html and then replace all the text
contents, similar to `getBy*`. This is a lot safer to do instead.
We could simply get the html content with `.innerHtml` but it doesn't
pretty print the DOM tree, which makes it very difficult to understand
the changes. There is a `prettyDOM` method in testing-library, but it's
added for debugging purposes and not really for testing.
@canova
canova requested a review from julienwMay 28, 2025 12:35
@codecov

codecovBot commented May 28, 2025

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.03%. Comparing base (6bc4be7) to head (6e9dd9a).
Report is 3 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #5475 +/- ##
=======================================
Coverage 86.03% 86.03% =======================================
Files 312 312 Lines 30364 30364 Branches 8296 8296 =======================================
Hits 26125 26125 Misses 3644 3644 Partials 595 595 

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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

@julienwjulienw left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

good idea, thanks

@canova
canova merged commit 1a1dc60 into firefox-devtools:mainMay 28, 2025
@canova
canova deleted the co2-test-fix branch May 28, 2025 13:43
@julienwjulienw mentioned this pull request Jun 10, 2025
julienw added a commit that referenced this pull request Jun 10, 2025
Here are the changes:
[Julien Wajsberg] Fix the compare view when using the same profile in both entries (#5469)
[Nazım Can Altınova] Change the tests that check co2 values so they don't fail on every co2 dependency update (#5475)
[Julien Wajsberg] Ignore the Symbol Store database cache when the user presses the resymbolication button (#5479)
[Markus Stange] In the profile format, store a shared stringArray for all threads (#5481)
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@canova@julienw