Skip to content

test: fix flakey snapshot normalization logic #7203

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Conversation

serhalp
Copy link
Collaborator

@serhalp serhalp commented Apr 14, 2025

Summary

This logic was normalizing integration test snapshots by replacing any sequence of five digits with 88888. The intent is to handle some randomness in test output.

It wasn't working some percentage of the time because randomly selected ports are sometimes four digits. You can see an example in this failed run: https://github.com/netlify/cli/actions/runs/14434953424/job/40474516002?pr=7202#step:10:1143.

While I was at it, I made it less much less generic and moved it into the only test suite that relies on it.

Copy link

github-actions bot commented Apr 14, 2025

📊 Benchmark results

Comparing with b60c7d3

  • Dependency count: 1,237 (no change)
  • Package size: 311 MB ⬇️ 0.00% decrease vs. b60c7d3
  • Number of ts-expect-error directives: 422 (no change)

@serhalp serhalp marked this pull request as ready for review April 14, 2025 13:33
@serhalp serhalp requested a review from a team as a code owner April 14, 2025 13:33
@@ -10,6 +10,12 @@ import { normalize } from './utils/snapshots.js'

const content = 'Hello World!'

// Normalize random ports
const normalizeSnapshot = (output, opts) =>
Copy link
Contributor

Choose a reason for hiding this comment

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

👏

Love to see this vs. one big world-normalizing function.

serhalp added 2 commits April 14, 2025 12:34
This logic was normalizing integration test snapshots by replacing any sequence of five
digits with `88888`. The intent is to handle some randomness in test output.

It wasn't working some percentage of the time because randomly selected ports are
sometimes _four_ digits. You can see an example in this failed run:
https://github.com/netlify/cli/actions/runs/14434953424/job/40474516002?pr=7202#step:10:1143.

While I was at it, I made it less much less generic and moved it into the only test suite
that relies on it.
@serhalp serhalp force-pushed the serhalp/cpla-2573-fix-flaky-integration-tests-remove-ci-automatic-test-suite-2 branch from 868d36d to 10c10ba Compare April 14, 2025 16:51
@serhalp serhalp enabled auto-merge (squash) April 14, 2025 16:51
@serhalp serhalp merged commit 1b00c77 into main Apr 14, 2025
52 checks passed
@serhalp serhalp deleted the serhalp/cpla-2573-fix-flaky-integration-tests-remove-ci-automatic-test-suite-2 branch April 14, 2025 17:22
serhalp added a commit that referenced this pull request Apr 14, 2025
serhalp added a commit that referenced this pull request Apr 14, 2025
serhalp added a commit that referenced this pull request Apr 15, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants