Add simple test CI

[k-sareen]

Heyo! Just added a very simple test CI using GitHub actions. A few things to note:

1. I'm not sure how the wasm target works as I tried to run the wasm tests on my local node installation and it simply skipped the execution
2. x86-macos can't be tested in CI unless you use a self-hosted runner or use some sort of emulation. I believe only x86_64 MacOS is available in GitHub actions (not even arm64 MacOS is available!)
3. I added the int target to all the native targets just for sanity -- you may want to make a separate one for it

Partially closes #92.

[k-sareen]

See here for CI results

P.S. GitHub really doesn't like progress spitting out a lot of text as my browser hangs if you try and open the "Build and Test" drop down for a target, so I'm unsure if this setup is feasible in the long term. Maybe progress for CLI, but some other simpler testing framework for CI?

EDIT: The CI files can be simplified since most targets are identical save the TEST_TARGETS environment variable. Maybe that'll be nicer?

[titzer]

Hey, Welcome back! Awesome that you took this up. The approach looks good so far.

P.S. GitHub really doesn't like progress spitting out a lot of text as my browser hangs if you try and open the "Build and Test" drop down for a target, so I'm unsure if this setup is feasible in the long term. Maybe progress for CLI, but some other simpler testing framework for CI?

EDIT: The CI files can be simplified since most targets are identical save the TEST_TARGETS environment variable. Maybe that'll be nicer?

Yes, I think it's good to have each tester have only the targets that it is able to run on the host. Otherwise it just compiles all tests for all targets and then skips running them. That also parallelizes the total work too.

[k-sareen]

Yes, I think it's good to have each tester have only the targets that it is able to run on the host. Otherwise it just compiles all tests for all targets and then skips running them. That also parallelizes the total work too.

Right. What I meant though was that currently the .sh files for all the targets are exactly the same except for the TEST_TARGETS environment variables. So we can remove the invididual .sh files in favour of 1 .sh file and set the TEST_TARGETS environment variable in the CI run step. Though I think the wasm target will probably need to have its own .sh file if it needs to do some specific setup. I guess it really depends on how much you care about "code duplication". I organized the CI files like this in case there were some platform/target specific tests or setup requirements, but it seems like there are none (at least for the 4 current targets).

Speaking of, could you give me an example of how the wasm target is supposed to be tested? I'll add it to the CI config then. I don't quite understand why the run-wasi@node config doesn't execute the wasm tests.

[k-sareen]

P.S. GitHub really doesn't like progress spitting out a lot of text as my browser hangs if you try and open the "Build and Test" drop down for a target, so I'm unsure if this setup is feasible in the long term. Maybe progress for CLI, but some other simpler testing framework for CI?

Okay I've made it so that we can set the progress arguments from an environment variable. The CI sets it the line mode (l), whereas by default it's in inline mode (i). See here for an example. My browser can at least open the "Build and Test" drop down now (though my PC fans still take-off!)

[titzer]

Speaking of, could you give me an example of how the wasm target is supposed to be tested? I'll add it to the CI config then. I don't quite understand why the run-wasi@node config doesn't execute the wasm tests.

Because wasi is more like a platform, and the individual codegen tests target wasm-js-test; there are no platform APIs necessary to run those tests. v3c generates a .js file that contains test expectations and runs a runner that loads and calls wasm. Those tests currently only work against d8 (i.e. the v8 shell) but they could as well work against node.

[k-sareen]

Speaking of, could you give me an example of how the wasm target is supposed to be tested? I'll add it to the CI config then. I don't quite understand why the run-wasi@node config doesn't execute the wasm tests.

Because wasi is more like a platform, and the individual codegen tests target wasm-js-test; there are no platform APIs necessary to run those tests. v3c generates a .js file that contains test expectations and runs a runner that loads and calls wasm. Those tests currently only work against d8 (i.e. the v8 shell) but they could as well work against node.

Right I see. I don't have any experience with wasm (or testing Virgil on wasm more precisely) to be able to change the wasm-js target to run on node. Could you point me to a direction so that I can run the wasm tests on node?

[k-sareen]

@titzer I've addressed the review comments. See here for an example of a CI run with the latest commit. I think the only thing left would be the wasm target. And also now that I think about it, maybe the summary or character mode (PROGRESS_ARGS=s or PROGRESS_ARGS=c) would be better than line mode for CI? What are your thoughts?

[titzer]

maybe the summary or character mode (PROGRESS_ARGS=s or PROGRESS_ARGS=c) would be better than line mode for CI?

I think line mode is fine, because you want to see if something gets stuck and where.

[titzer]

Could you point me to a direction so that I can run the wasm tests on node?

Sure. The -target=wasm-js-test causes .wasm (code) and a .js file (expected input/output) to be generated by the compiler and the test/bin/test-wasm-js script runs d8 with the wasm-js-tester.js input file that loads wasm and expected inputs and outputs. I think the wasm-js-tester.js would need a little adaptation but the other things probably would work.

[k-sareen]

Sure. The -target=wasm-js-test causes .wasm (code) and a .js file (expected input/output) to be generated by the compiler and the test/bin/test-wasm-js script runs d8 with the wasm-js-tester.js input file that loads wasm and expected inputs and outputs. I think the wasm-js-tester.js would need a little adaptation but the other things probably would work.

Right. I already see there are some d8 specific comments in the file. I'll have to investigate node+wasm then, it may have to be rewritten a bit.

[k-sareen]

Alright I have it working @titzer. Should I keep the original d8 script (and make a new wasm-nodejs target) or replace it with the node js one?

[k-sareen]

Alright I've opted to do the former. I've added a test-wasm-js-node script that is used if d8 is not found. See the runs here: https://github.com/k-sareen/virgil/actions/runs/3612764595. It works! Let me know if any other changes are required @titzer.

[k-sareen]

By the way, as mentioned above, passing the CI depends on if the script exits with an exit code of 0. It seems like, however, that failing a test (for example by forcing a failure by changing the test expected output) does not make the script fail as a whole. I think this needs to be fixed as well.

Case in point:

TEST_TARGETS="wasm-js" aeneas test enums

TEST_HOST=x86-linux
TEST_TARGETS="wasm-js"
TEST_CACHE=
V3C_STABLE=/home/$USER/git/virgil/bin/stable/x86-linux/Aeneas
V3C_OPTS=""
PROGRESS_ARGS="i"
AENEAS_TEST="auto"
Compiling (/home/$USER/git/virgil/bin/stable/x86-linux/Aeneas -> /tmp/$USER/virgil-test/aeneas/bootstrap/x86-linux/Aeneas)...
  1948464 /tmp/$USER/virgil-test/aeneas/bootstrap/x86-linux/Aeneas
--------------------------------------------------------------------------------
(/tmp/$USER/virgil-test/aeneas/bootstrap/x86-linux/Aeneas) enums
  Parser                      | 38 of 38 passed
  Semantic                    | 210 of 210 passed
  Compiling     wasm-js       | 134 of 134 passed
    running                   |
big_set_sub00: fail: Run 0 failed: Error: expected 181, but got 182
133 of 134 passed 1 failed
Compiling (/tmp/$USER/virgil-test/aeneas/bootstrap/x86-linux/Aeneas -> /tmp/$USER/virgil-test/aeneas/current/x86-linux/Aeneas)...
  1948464 /tmp/$USER/virgil-test/aeneas/current/x86-linux/Aeneas
  bin/current == bin/bootstrap ok
virgil testing-ci *1 !1 ?1 ❯ echo $?
0

[k-sareen]

Alright -- latest commit should have it fixed now @titzer. A failing test should result in a non-zero exit code. See here for a sample run.

I've made the CI such that it exits with a failure if the bootstrap check fails even if all tests pass for both versions. I don't know if you want to keep this behaviour or not as a theoretical improvement to the register allocator, for example, would make the CI fail. Though it should be noted that you can configure your repository such that you can merge regardless of CI status (it might be the default already?).

Here's an example where I've randomly changed a test's expected value to force a failure:

TEST_TARGETS="wasm-js" ./test/all.bash enums

TEST_HOST=x86-linux
TEST_TARGETS="wasm-js"
TEST_CACHE=
V3C_STABLE=/home/$USER/git/virgil/bin/stable/x86-linux/Aeneas
V3C_OPTS=""
PROGRESS_ARGS="i"
AENEAS_TEST="auto"
Compiling (/home/$USER/git/virgil/bin/stable/x86-linux/Aeneas -> /tmp/$USER/virgil-test/aeneas/bootstrap/x86-linux/Aeneas)...
  1948464 /tmp/$USER/virgil-test/aeneas/bootstrap/x86-linux/Aeneas
--------------------------------------------------------------------------------
(/tmp/$USER/virgil-test/aeneas/bootstrap/x86-linux/Aeneas) enums
  Parser                      | 38 of 38 passed
  Semantic                    | 210 of 210 passed
  Compiling     wasm-js       | 134 of 134 passed
    running                   |
big_set_sub00: fail: Run 0 failed: Error: expected 181, but got 182
133 of 134 passed 1 failed
Compiling (/tmp/$USER/virgil-test/aeneas/bootstrap/x86-linux/Aeneas -> /tmp/$USER/virgil-test/aeneas/current/x86-linux/Aeneas)...
  1948464 /tmp/$USER/virgil-test/aeneas/current/x86-linux/Aeneas
  bin/current == bin/bootstrap ok
virgil testing-ci *1 !1 ?1 ❯ echo $?
1

[titzer]

Great work, thanks Kunal!