Skip to content

[variant] Add missing variant in ID generation - #809

Merged
trunk-io[bot] merged 3 commits into
mainfrom
gabe/trunk-16936-variant-ids-in-internalbin-are-incorrect
Nov 5, 2025
Merged

trunk-io[bot] merged 3 commits into
mainfrom
gabe/trunk-16936-variant-ids-in-internalbin-are-incorrect

Conversation

@gnalh

@gnalh gnalh commented Nov 4, 2025

Copy link
Copy Markdown
Contributor

Also address xcresult not supporting variants

@trunk-io

trunk-io Bot commented Nov 4, 2025 •

Copy link
Copy Markdown

😎 Merged successfully - details.

@codecov-commenter

codecov-commenter commented Nov 5, 2025 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.59259% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.10%. Comparing base (6654b79) to head (9b91704).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
cli/src/context.rs 0.00% 3 Missing ⚠️
context/src/junit/parser.rs 96.10% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #809      +/-   ##
==========================================
+ Coverage   72.79%   73.10%   +0.31%     
==========================================
  Files          72       72              
  Lines       15960    16024      +64     
==========================================
+ Hits        11618    11715      +97     
+ Misses       4342     4309      -33     

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

Comment thread context/src/junit/parser.rs Outdated
});
}
} else {
gen_info_id(

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.

We have a generate_info_id_variant_wrapper in the same file as gen_info_id that should encapsulate this logic.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Will update

@mb1206 mb1206 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

seems good to me! this would be something that would only affect new tests right? or would it regenerate ids for existing tests?

@gnalh

gnalh commented Nov 5, 2025

Copy link
Copy Markdown
Contributor Author

seems good to me! this would be something that would only affect new tests right? or would it regenerate ids for existing tests?

These are the same IDs that are already being used in the ETL, we just haven't flipped the switch yet to use these internal.bin files yet.

@gnalh
gnalh requested review from cmillar-trunk and mb1206 November 5, 2025 18:53
@gnalh
gnalh force-pushed the gabe/trunk-16936-variant-ids-in-internalbin-are-incorrect branch from 8918951 to 9b91704 Compare November 5, 2025 20:10
@trunk-io
trunk-io Bot merged commit 99314a0 into main Nov 5, 2025
18 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants