Skip to content

Add variants to jobs #1008

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 15, 2025
Merged

Add variants to jobs #1008

merged 3 commits into from
Apr 15, 2025

Conversation

cmillar-trunk
Copy link
Contributor

Adds variants to the staging jobs. Only applies to staging, and uses a branch to avoid early public availability.

Adds variants to the staging jobs. Only applies to staging, and uses a
branch to avoid early public availability.
Copy link

trunk-io bot commented Apr 11, 2025

⏱️ 11m total CI duration on this PR
Job Cumulative Duration Recent Runs
CodeQL-Build 4m 🟩🟩🟩
Action Tests 3m 🟩🟩
Trunk Check runner [linux] 2m 🟩🟩🟩
Repo Tests / Plugin Tests 2m 🟩🟩🟩
Detect changed files 12s 🟩🟩🟩
Aggregate Test Results 7s 🟩🟩🟩

settingsfeedbackdocs ⋅ learn more about trunk.io

Copy link
Collaborator

@TylerJang27 TylerJang27 left a comment

Choose a reason for hiding this comment

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

You'll probably also want to wire in changing places where JEST_JUNIT_SUITE_NAME is set, otherwise the variants will only show up under different tests still

Copy link

trunk-staging-io bot commented Apr 11, 2025

Static BadgeStatic BadgeStatic BadgeStatic Badge

View Full Report ↗︎Docs

Copy link

trunk-io bot commented Apr 11, 2025

Static BadgeStatic BadgeStatic BadgeStatic Badge

View Full Report ↗︎Docs

@cmillar-trunk
Copy link
Contributor Author

You'll probably also want to wire in changing places where JEST_JUNIT_SUITE_NAME is set, otherwise the variants will only show up under different tests still

I'm not sure I understand this? Why does that matter, and what do you mean by show up under different tests?

@gnalh gnalh requested a review from TylerJang27 April 14, 2025 16:27
@TylerJang27
Copy link
Collaborator

Big fan of variants and I want to get it set up here. Right now we set JEST_JUNIT_SUITE_NAME manually in order to mimic the concept of variants. This means that when we run a test on linux and mac, they show up as different tests

So if the desire of this change is for this to show up as one Testing linter osv-scanner main Snapshots instead of separate Testing linter osv-scanner <os> main Snapshots, you should modify the use of JEST_JUNIT_SUITE_NAME to omit the OS piece. This will create a new test to track this, but I think that's fine

@cmillar-trunk
Copy link
Contributor Author

I see, will do. You're correct that it'll create a new test, but adding a variant does that anyway, so that's already a thing.

@cmillar-trunk
Copy link
Contributor Author

So if the desire of this change is for this to show up as one Testing linter osv-scanner main Snapshots instead of separate Testing linter osv-scanner <os> main Snapshots, you should modify the use of JEST_JUNIT_SUITE_NAME to omit the OS piece. This will create a new test to track this, but I think that's fine

@TylerJang27 Ah, gotcha. Since we're only testing this on staging, and the uploader runs for the same tests on both staging and prod, I'd prefer to leave it as-is to not mess with prod for now, if that's alright?

@cmillar-trunk cmillar-trunk marked this pull request as ready for review April 15, 2025 19:55
Copy link
Collaborator

@TylerJang27 TylerJang27 left a comment

Choose a reason for hiding this comment

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

Sounds good, that's fine by me

@cmillar-trunk cmillar-trunk merged commit 8c2cb9f into main Apr 15, 2025
13 checks passed
@cmillar-trunk cmillar-trunk deleted the christian/add-variants-to-jobs branch April 15, 2025 20:21
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.

2 participants