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 stdlib tests when targeting WebAssembly/WASI #39519

Merged
merged 1 commit into from
Jan 12, 2022

Conversation

MaxDesiatov
Copy link
Contributor

This change adds support for WASI in stdlib tests. Some tests that expect a crash to happen had to be disabled, since there's currently no way to observe such crash from a WASI host.

Related to SR-9307.

@MaxDesiatov
Copy link
Contributor Author

@swift-ci please smoke test

@MaxDesiatov
Copy link
Contributor Author

@swift-ci please test Windows platform

@MaxDesiatov MaxDesiatov changed the title test: add handling for WebAssembly/WASI test: fix stdlib tests when targeting WebAssembly/WASI Sep 30, 2021
@MaxDesiatov
Copy link
Contributor Author

@swift-ci please test Windows platform

Copy link
Member

@compnerd compnerd left a comment

Choose a reason for hiding this comment

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

What exactly does WASI do if a trap is encountered? Can we simply hook the system to observe the trap? This seems like something we should address in the compiler rather than workaround in each of the tests.

@@ -335,6 +337,36 @@ public func evaluateObservationsAllEqual<T : Equatable>(_ observations: [T])
return .pass
}

// WebAssembly/WASI doesn't support multi-threading yet
Copy link
Member

Choose a reason for hiding this comment

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

Can we not just mark the test as UNSUPPORTED on OS=wasi instead?

Copy link
Contributor Author

@MaxDesiatov MaxDesiatov Sep 30, 2021

Choose a reason for hiding this comment

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

This isn't a lit test. Can any test in StdlibUnittest be marked as UNSUPPORTED?

Copy link
Member

Choose a reason for hiding this comment

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

I guess compnerd said we can put unsupported statements for each test case, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, not sure what you mean here, could you share an example?

@MaxDesiatov
Copy link
Contributor Author

according to the spec https://webassembly.github.io/spec/core/intro/overview.html:

Traps
Under some conditions, certain instructions may produce a trap, which immediately aborts execution. Traps cannot be handled by WebAssembly code, but are reported to the outside environment, where they typically can be caught.

This means that traps shut down the whole WebAssembly VM, and it's up to the host environment to restart it or not. But I don't think there's anything that the compiler can do to intercept such trap.

@MaxDesiatov
Copy link
Contributor Author

CC @kateinoigakukun

@MaxDesiatov
Copy link
Contributor Author

@swift-ci please test Windows platform

@DougGregor
Copy link
Member

@swift-ci please smoke test

@DougGregor
Copy link
Member

@swift-ci please test Windows

@MaxDesiatov MaxDesiatov merged commit 372ada0 into main Jan 12, 2022
@MaxDesiatov MaxDesiatov deleted the maxd/if-os-wasi branch January 12, 2022 14:24
@MaxDesiatov
Copy link
Contributor Author

Tests were fully passing on all platforms. When merging I assumed no further objections to be resolved were present, let me know otherwise.

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.

None yet

4 participants