Skip to content

Fix a typo in run_all_no_compressed_oop #306

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 1 commit into from
Apr 28, 2025
Merged

Conversation

wks
Copy link
Contributor

@wks wks commented Apr 25, 2025

This function should take one argument, but it used $2 instead. In previous runs, invocations of run_all_no_compressed_oop have failed silently without making the GitHub-level CI test case ci-test-only-normal-no-compressed-oops fail.

This PR fixes the typo.

This function should take one argument, but it used $2 instead.
In previous runs, invocations of run_all_no_compressed_oop have failed
silently without making the GitHub-level CI test case
ci-test-only-normal-no-compressed-oops fail.

This PR fixes the typo.
@wks wks requested a review from qinsoon April 25, 2025 07:42
@wks
Copy link
Contributor Author

wks commented Apr 25, 2025

This is a strange bug. Previously the test case about compressed oops have been "successful" so far, which should not happen.

Take https://github.com/mmtk/mmtk-openjdk/actions/runs/14508960198/job/40704006542#step:7:37 as an example. The error ./.github/scripts/common.sh: line 31: -XX:-UseCompressedOops: syntax error in expression (error token is ":-UseCompressedOops") appeared 7 times in that GitHub job, but the whole job was successful.

On the Bash level, if a syntax error happens in $(( blah * blah)), the enclosing bash function will return 1. But even with set -xe, failure in one command won't automatically terminate the whole script.

This PR should fix this particular bug, but we should think about how to make our test scripts more robust against such failures.

@wks wks merged commit b77e0b9 into mmtk:master Apr 28, 2025
55 of 57 checks passed
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.

2 participants