Add a GitHub actions check test262.properties#2052
Conversation
Check if the test262.properties file was updated correctly using the built-in tools, as opposed to being edited manually. Also create the "tools" directory, where I suspect other handy scripts can be kept.
rPraml
left a comment
rPraml
left a comment
There was a problem hiding this comment.
Great!
Have you considered whether it makes sense for the Github action to commit the file?
| - name: Check out test262 | ||
| # We don't actually want all the history for this part | ||
| run: git submodule update --init --single-branch | ||
| - name: Set up Java |
There was a problem hiding this comment.
If you do not rely on java 11, you can skip this step. Ubuntu-latest has already java 21 on board
|
OK -- I'm going to have to muck about a bit more just to make it work (and probably not more today). My plan is to add a few dummy commits to see if it really works, and if it does move this logic into the main GitHub action, because it doesn't make a lot of sense for this test to run if the main tests are failing. (Unless we can get Actions to execute the tests in some sort of dependency order, which I don't recall if we can do.) I'm not sure how I feel about auto-commit -- would it mess up everyone's GH workloads? If not then we can check it out. |
|
After mucking around for a bit I am just adding a new step to the main CI job which runs test262.properties and checks to see if it needs a re-run. That will catch situations like forgetting to edit the properties file when fixing tests, or manual edits. If it works well we can look at an auto-commit check. |
| run: echo "hash=${{ hashFiles('./tests/testsrc/test262.properties') }}" >> $GITHUB_OUTPUT | ||
| - name: Compare test262 results | ||
| run: | | ||
| if [ "${{ steps.hashBefore.outputs.hash }}" != "${{ steps.hashAfter.outputs.hash }}" ]; then |
There was a problem hiding this comment.
You might check for modification with git diff / git status here.
How long does this step take?
As long as I understand, this step executes ALL 262 tests... And the java 11 test run executes then again (but only the not excluded ones) - so it should be possible to exclude them (or regenerate the props file in the java 11 test run)
I'm fine with your changes and the above are suggestions, that can (or not) be done in further PRs 👍
|
Thanks -- I get it -- I'd like to take a conservative approach and re-check the test properties only after we know that there are no big test failures. It might be time to see if we can come up with some sort of a pipeline of jobs, though, so we can look at that next! |
2 participants
Check if the test262.properties file was updated correctly using the
built-in tools, as opposed to being edited manually.
Also create the "tools" directory, where I suspect other handy scripts
can be kept.