Enable WASI tests on Windows CI #2699
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zoraaver
commented
Nov 1, 2023
zoraaver
force-pushed
the
enable-wasi-tests
branch
from
486bedf
to
4046d63
Compare
zoraaver
commented
Nov 1, 2023
zoraaver
force-pushed
the
enable-wasi-tests
branch
3 times, most recently
from
ba70ea5
to
19af6f3
Compare
wenyongh
reviewed
Nov 2, 2023
| DWORD win_error = GetLastError(); | ||
| if (win_error != ERROR_IO_PENDING) { | ||
| error = convert_windows_error_code(win_error); | ||
| break; |
There was a problem hiding this comment.
Is this an error, shall we check whether i == iovcnt or error after the for loop to check whether there was an error?
There was a problem hiding this comment.
Not sure if I'm understanding fully but even if there is an error for one of the read operations, we should still wait on the previous successful read operations. successful_read_count is used to make sure we only wait on the operations which were started successfully.
| GetOverlappedResult(handle, &read_operations[j], | ||
| &bytes_transferred, true); | ||
| } | ||
| break; |
| DWORD win_error = GetLastError(); | ||
| if (win_error != ERROR_IO_PENDING) { | ||
| error = convert_windows_error_code(win_error); | ||
| break; |
zoraaver
force-pushed
the
enable-wasi-tests
branch
4 times, most recently
from
d60fbff
to
da5ce7f
Compare
In order to start running WASI tests in CI on Windows, add the ability to filter WASI tests in the test runner scripts. The WASI libc implementation is not yet complete on Windows so not all the tests are passing. Once the implementation is complete, we can remove the filter (although it's probably still useful to keep the filtering functionality in the test runner either way).
zoraaver
force-pushed
the
enable-wasi-tests
branch
6 times, most recently
from
29656c6
to
1997ef6
Compare
zoraaver
commented
Nov 2, 2023
| #define PATH_SEPARATORS "/\\" | ||
| #else | ||
| #define PATH_SEPARATORS "/" | ||
| #endif |
There was a problem hiding this comment.
Not strictly related to this PR but I realized we'll need it on Windows so paths with backslashes are split into components properly. It would probably be good to verify the behaviour of backslashes with an additional WASI test.
zoraaver
force-pushed
the
enable-wasi-tests
branch
6 times, most recently
from
f0ae5aa
to
ced80c9
Compare
loganek
reviewed
Nov 2, 2023
loganek
reviewed
Nov 2, 2023
wenyongh
reviewed
Nov 3, 2023
Most of the WASI filesystem tests require at least creating/deleting a file to test filesystem functionality so some additional filesystem APIs have been implemented on Windows so we can test what has been implemented so far. For those WASI functions which haven't been implemented, we skip the tests. These will be implemented in a future PR after which we can remove the relevant filters. Additionally, in order to run the WASI socket and thread tests, we need to install the wasi-sdk in CI and build the test source code prior to running the tests.
zoraaver
force-pushed
the
enable-wasi-tests
branch
from
ced80c9
to
8091329
Compare
wenyongh
merged commit 13875f4
into
bytecodealliance:dev/wasi-libc-windows
383 checks passed
victoryang00
pushed a commit
to victoryang00/wamr-aot-gc-checkpoint-restore
that referenced
this pull request
May 27, 2024
Most of the WASI filesystem tests require at least creating/deleting a file to test filesystem functionality so some additional filesystem APIs have been implemented on Windows so we can test what has been implemented so far. For those WASI functions which haven't been implemented, we skip the tests. These will be implemented in a future PR after which we can remove the relevant filters. Additionally, in order to run the WASI socket and thread tests, we need to install the wasi-sdk in CI and build the test source code prior to running the tests.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've split this up change into two commits to make it (hopefully!) easier to review.