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: mark two WASI tests flaky on windows #33403

Closed
wants to merge 1 commit into from

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented May 14, 2020

These tests have become flaky on Windows in the CI. The failures do not appear to be consistent, or even in the same parts of the tests. They do seem to be related to trying to open or stat files though.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

These tests have become flaky on Windows in the CI. The failures
do not appear to be consistent, or even in the same parts of the
tests. They do seem to be related to trying to open or stat
files though.
@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label May 14, 2020
@cjihrig
Copy link
Contributor Author

cjihrig commented May 14, 2020

FWIW, I'm experimenting with applying the revert from #33364 to see if it makes a difference.

From some testing last night, I saw failures on 2 out of 5 runs - both on Windows compiled with VS 2019. With the revert applied, I'm currently seeing success for 3 out of 3 4 out of 4 runs, but I'm going to make a few more runs.

@cjihrig
Copy link
Contributor Author

cjihrig commented May 14, 2020

I ran test-wasi through a stress test on Windows with the revert applied. It did fail two times, but it certainly seems a lot less flaky with the revert applied.

Across the various Windows machines, the test ran 500 times. It failed once on win2012r2-vs2019 in a WASI stat call. It failed in the same way once on win2016-vs2017 as well. I'll have to do some more debugging but I'm OK with labeling these as flaky for now since there wasn't a 100% pass rate. This might also be a data point in favor of landing #33364.

EDIT: I also plan to look into the nature of the stat failure since Windows is prone to that IIRC from other issues.

@cjihrig cjihrig deleted the flake branch May 17, 2020 15:25
@cjihrig cjihrig mentioned this pull request May 17, 2020
2 tasks
addaleax pushed a commit that referenced this pull request May 19, 2020
Notable changes:

- A `DEBUG()` macro and `UVWASI_DEBUG_LOG` build option have been
  added to improve debugging.
- Path length restrictions have been removed across the codebase.
- Initial support for `poll_oneoff()` has been added on all
  platforms. The implementation is based on `uv_poll_t`'s.
- A new `uvwasi_size_t` has been introduced across the WASI system
  call API. This provides consistent 32-bit `size_t`'s.
- The cmake test targets are now only generated if uvwasi is the
  root project to avoid conflicts with targets from embedders.
- `uv.h` has been removed from the public headers.
- A serialization/deserialization API has been added to simplify
  the process of working with WASM memory. This also hides many
  WASI <--> WASM interfacing implementation details from
  embedders.
- A memory corruption bug on Windows related to path resolution
  has been fixed.

PR-URL: #33445
Fixes: #33403
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
codebytere pushed a commit that referenced this pull request May 21, 2020
Notable changes:

- A `DEBUG()` macro and `UVWASI_DEBUG_LOG` build option have been
  added to improve debugging.
- Path length restrictions have been removed across the codebase.
- Initial support for `poll_oneoff()` has been added on all
  platforms. The implementation is based on `uv_poll_t`'s.
- A new `uvwasi_size_t` has been introduced across the WASI system
  call API. This provides consistent 32-bit `size_t`'s.
- The cmake test targets are now only generated if uvwasi is the
  root project to avoid conflicts with targets from embedders.
- `uv.h` has been removed from the public headers.
- A serialization/deserialization API has been added to simplify
  the process of working with WASM memory. This also hides many
  WASI <--> WASM interfacing implementation details from
  embedders.
- A memory corruption bug on Windows related to path resolution
  has been fixed.

PR-URL: #33445
Fixes: #33403
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
codebytere pushed a commit that referenced this pull request Jun 9, 2020
Notable changes:

- A `DEBUG()` macro and `UVWASI_DEBUG_LOG` build option have been
  added to improve debugging.
- Path length restrictions have been removed across the codebase.
- Initial support for `poll_oneoff()` has been added on all
  platforms. The implementation is based on `uv_poll_t`'s.
- A new `uvwasi_size_t` has been introduced across the WASI system
  call API. This provides consistent 32-bit `size_t`'s.
- The cmake test targets are now only generated if uvwasi is the
  root project to avoid conflicts with targets from embedders.
- `uv.h` has been removed from the public headers.
- A serialization/deserialization API has been added to simplify
  the process of working with WASM memory. This also hides many
  WASI <--> WASM interfacing implementation details from
  embedders.
- A memory corruption bug on Windows related to path resolution
  has been fixed.

PR-URL: #33445
Fixes: #33403
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants