-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
chore: Fix JUnit tests not running #38208
chore: Fix JUnit tests not running #38208
Conversation
Signed-off-by: Arthur Silva Sens <[email protected]>
@@ -289,15 +289,15 @@ jobs: | |||
if: startsWith( matrix.go-version, '1.23' ) != true | |||
run: make gotest GROUP=${{ matrix.group }} | |||
- name: Run Unit Tests With JUnit and Coverage | |||
if: startsWith( matrix.go-version, '1.23' ) # only run junit/coverage on one version | |||
if: startsWith( matrix.go-version, '1.23' ) != true # only run junit/coverage on one version |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this was missed out in the previous PRs (Go1.22 -> Go1.23)... startsWith( matrix.go-version, '1.23' )
is correct, startsWith( matrix.go-version, '1.23' ) != true
is wrong, so instead you would want to change the latter to the former everywhere else
cc @mx-psi can correct me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think Yang is right here, yep
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like there's something we're still missing 🤔
Run Unit Tests
, as it's configured today, should only run when the go version doesn't start with 1.23. Therefore it should only run for go 1.24. This is not the behavior I'm seeing.:
Now for the JUnit tests, it is configured to run when it starts with 1.23, but it's not running for any of the go versions.
My hypothesis is that startsWith(...)
is not returning anything, and therefore != true
is triggering the unit tests because empty is different than true
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could try and print the result of the expression in the Github Actions logs and we can check it on this PR
Signed-off-by: Arthur Silva Sens <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this still relevant?
Thanks for the ping, not anymore! |
Addresses a silly mistake that we missed in the last PR.
See #38177 (comment) for details