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

test: fix some wasi_testsuite cases #6343

Merged
merged 2 commits into from
May 5, 2023

Conversation

abrown
Copy link
Collaborator

@abrown abrown commented May 4, 2023

After thinking about this some more, I took another look at fixing some issues with the test cases. By doing the following, many more test cases now pass:

  • this change cleans up garbage files created by the test
  • this change also fixes how directories are mapped, using --mapdir instead of --dir

See the commit messages for more detail.

abrown added 2 commits May 4, 2023 14:25
When running `cargo run wasi_testsuite -- --nocapture`, some test cases
generate files and folders that need to be removed for subsequent test
runs to pass. These paths are all suffixed with `*.cleanup` so we walk
the suite directory before and after execution to clean up these files.
This change fixes how directories are mapped from the host environment
to the WebAssembly guest when the `wasi_testsuite` test is run.
Previously, Wasmtime's `--dir` flag was used which expects the passed
directory to be in the current working directory. In this case, though,
the directories were buried down `tests/wasi-testsuite/wasi-common/...`
so the correct flag is `--mapdir`. Switching to this flag allows a large
number of the ignored tests to now pass.
@abrown abrown requested a review from a team as a code owner May 4, 2023 21:28
@abrown abrown requested review from fitzgen and removed request for a team May 4, 2023 21:28
cmd.arg("--dir");
cmd.arg(parent_dir.join(dir));
cmd.arg("--mapdir");
cmd.arg(format!("{}::{}", dir, parent_dir.join(dir).display()));
Copy link
Member

Choose a reason for hiding this comment

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

Instead of passing the raw directory in the spec test suite to each test, could the directory perhaps be copied to a temporary location and have the test run against that? That way the temporary directory clean up would removing any extra files and the clean_garbage filtering may not be necessary?

Alternatively though explicitly cleaning out files I think is ok, and if this is how the test suite runs upstream I think it's fine to mirror that as well.

Copy link
Collaborator Author

@abrown abrown May 4, 2023

Choose a reason for hiding this comment

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

Well, right now I believe the other wasi-testsuite test runner (the Python one) expects to clean up any *.cleanup files so I propose we stick with that for now. I opened WebAssembly/wasi-testsuite#74 to actually write down what a test runner should do and I added a comment there about this. How about we change this logic once we decide what to do there?

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good to me 👍

@abrown abrown added this pull request to the merge queue May 5, 2023
Merged via the queue into bytecodealliance:main with commit b2118a9 May 5, 2023
@abrown abrown deleted the fix-some-wasi-tests branch May 5, 2023 01:41
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