Skip to content

Comments

test: use a fixed length name for temp directories#2544

Merged
G-Rath merged 1 commit intogoogle:mainfrom
ackama:test/use-fixed-length
Feb 23, 2026
Merged

test: use a fixed length name for temp directories#2544
G-Rath merged 1 commit intogoogle:mainfrom
ackama:test/use-fixed-length

Conversation

@G-Rath
Copy link
Collaborator

@G-Rath G-Rath commented Feb 22, 2026

Most of the time os.MkdirTemp creates directories with the same number of characters, but its not required to and will occasionally has one less character than what is common, which can result in the table output being slightly different.

This is compounded by us replacing the temp directory with a fixed string <tempdir> marker, as it ends up looking like the table marks are just randomly changing for no reason 🤦

Copy link
Collaborator

@another-rex another-rex 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 technically we're meant to check whether that random folder already exists and generate a new random name if it does to avoid conflicts.

But what are the chances?

@G-Rath
Copy link
Collaborator Author

G-Rath commented Feb 22, 2026

yeah that was my thinking - it should be easy to put in a loop for that if it ever happens, but it should already be pretty small and arguably by the same logic it's technically possible to generate directories with the same name that already exist on every loop, at which point 🤷

so line has to be somewhere right

@G-Rath G-Rath force-pushed the test/use-fixed-length branch from 4f87d65 to 3dbe266 Compare February 23, 2026 00:22
@G-Rath G-Rath enabled auto-merge (squash) February 23, 2026 00:29
@codecov-commenter
Copy link

codecov-commenter commented Feb 23, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.90%. Comparing base (a750891) to head (3dbe266).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2544      +/-   ##
==========================================
- Coverage   67.92%   67.90%   -0.02%     
==========================================
  Files         172      172              
  Lines       13395    13401       +6     
==========================================
+ Hits         9098     9100       +2     
- Misses       3581     3583       +2     
- Partials      716      718       +2     

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

@G-Rath G-Rath merged commit 614e266 into google:main Feb 23, 2026
17 checks passed
@G-Rath G-Rath deleted the test/use-fixed-length branch February 23, 2026 00:34
@G-Rath
Copy link
Collaborator Author

G-Rath commented Feb 23, 2026

ok so turns out this wasn't needed because we're outputting absolute paths which will differ in length anyway due to different OS/path setups 🙃

Sign up for free to 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.

4 participants