Skip to content
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

gha: add wasi-testsuite #1062

Merged
merged 1 commit into from
Jan 25, 2023

Conversation

evacchi
Copy link
Contributor

@evacchi evacchi commented Jan 25, 2023

Checkout wasi-testsuite from tetratelabs/wasi-testsuite@wazero branch then launch the tests using the wazero adapter.

It looks fast enough it could run at each PR.

  • Windows looks broken, but I think it may be an issue with -mount syntax (Windows paths may contain :) . I disabled it for now.

Signed-off-by: Edoardo Vacchi evacchi@users.noreply.github.com


wasi-testsuite:
name: wasi-testsuite
runs-on: ubuntu-20.04
Copy link
Member

Choose a reason for hiding this comment

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

let's run mac and windows as well!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh! 😬

Copy link
Contributor Author

Choose a reason for hiding this comment

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

is that runs-on: ${{ matrix.os }} ?

Copy link
Member

Choose a reason for hiding this comment

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

yep

Copy link
Contributor Author

Choose a reason for hiding this comment

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

missing Python module on macOS:

  • ModuleNotFoundError: No module named 'colorama'
    18

path syntax error on windows?

  • go install ./cmd/wazero
    3

@evacchi evacchi force-pushed the gha/wasi-testsuite branch from 64311b5 to 26b117a Compare January 25, 2023 10:27
@@ -167,3 +167,43 @@ jobs:
unicode \
unicode/utf16 \
unicode/utf8


wasi-testsuite:
Copy link
Member

Choose a reason for hiding this comment

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

You have to install the python dependencies via pip install -r requirements/dev.txt and please configure the proper caching for them`

https://github.com/WebAssembly/wasi-testsuite/blob/e5bcece941f66f8f6c845a9add32f9f99e444a2c/.github/workflows/test-runner.yml#L31

Copy link
Contributor Author

Choose a reason for hiding this comment

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

should be ok now

@evacchi evacchi force-pushed the gha/wasi-testsuite branch from 26b117a to d859023 Compare January 25, 2023 16:50
@evacchi
Copy link
Contributor Author

evacchi commented Jan 25, 2023

I think I managed to run the scripts on Windows as well, but it was not trivial. The "fix" I tried is to set an env variable to make sure the runners are able to find bash (which is not really on the PATH). see https://github.com/tetratelabs/wasi-testsuite/pull/1

It looks like some tests are not passing but it is likely to be the -mount directive not being parsed/passed correctly

@evacchi evacchi marked this pull request as draft January 25, 2023 17:07
@evacchi evacchi force-pushed the gha/wasi-testsuite branch from d859023 to f6d895e Compare January 25, 2023 18:53
Copy link
Contributor

@codefromthecrypt codefromthecrypt left a comment

Choose a reason for hiding this comment

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

ps when we get to windows, echo "EXTRA_ENV_VAR=value" >> $GITHUB_ENV in a conditional step will be better than repeating the latter.

runs-on: ${{ matrix.os }}
strategy:
matrix:
os: [ubuntu-20.04, macos-12, windows-2022]
Copy link
Contributor

Choose a reason for hiding this comment

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

switch windows-2022 to a TODO comment

- name: Initialize Python environment
uses: actions/setup-python@v4
with:
python-version: '3.10'
Copy link
Contributor

Choose a reason for hiding this comment

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

comment if this can be any version of python (e.g. . # latest version of python 3 if it isn't specific)

@evacchi evacchi force-pushed the gha/wasi-testsuite branch from f6d895e to a125191 Compare January 25, 2023 19:08
@evacchi evacchi marked this pull request as ready for review January 25, 2023 19:12
@evacchi
Copy link
Contributor Author

evacchi commented Jan 25, 2023

ps when we get to windows, echo "EXTRA_ENV_VAR=value" >> $GITHUB_ENV in a conditional step will be better than repeating the latter.

yeah but will that work on Windows?

Signed-off-by: Edoardo Vacchi <evacchi@users.noreply.github.com>
@evacchi evacchi force-pushed the gha/wasi-testsuite branch from a125191 to 851523b Compare January 25, 2023 19:14
@codefromthecrypt
Copy link
Contributor

actually I wonder if this would have worked on windows vs setting that ENV variable. In func-e we force bash for consistency https://github.com/tetratelabs/func-e/blob/master/.github/workflows/packaging.yaml#L34

@codefromthecrypt codefromthecrypt merged commit affcf6c into tetratelabs:main Jan 25, 2023
@codefromthecrypt
Copy link
Contributor

Thanks for this!

@evacchi
Copy link
Contributor Author

evacchi commented Jan 25, 2023

Unfortunately no I tried that. Apparently the "right" bash is not on the PATH. An alternative fix may be to fiddle with PATH too but I wonder how much pain it is to do it in GHA, on Windows, x-platform. I tried to figure it out and it looked complicated

@evacchi evacchi deleted the gha/wasi-testsuite branch January 25, 2023 19:30
@codefromthecrypt
Copy link
Contributor

Thanks for trying! In any case if we default to bash we can use the append syntax to add special variables, even if we yeah still need to add those. e.g. echo "$WIX\\bin" >> $GITHUB_PATH works now in func-e's GHA

@codefromthecrypt
Copy link
Contributor

or we can use normal template variables. In any case, setting variables is the easier part.

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.

3 participants