-
Notifications
You must be signed in to change notification settings - Fork 569
Fix or skip newly added Go compiler tests in 1.16 #1009
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
Fix or skip newly added Go compiler tests in 1.16 #1009
Conversation
If required modules are not available (e.g. in a browser), we fall back to console.log(). This also allows us to pass fixedbugs/issue22326.go test in Go 1.16.
These tests cover nuances of interaction between unsafe and GC, which are not applicable to the JavaScript runtime. Note: fixedbugs/issue24491a.go seems to panic where I don't think it should, is likely related to issue gopherjs#1001.
These tests rely on features GopherJS currently doesn't support.
Upon a cursory look, the test seems to panic when it is expected to, but the panic message is formatted differently, hence the test failure.
This mirrors the upstream golang/go#31546 and allows `fixedbugs/issue31546.go` to pass.
This test manifested a different issue in GopherJS compiler, which requires more research than I currently have time for. See gopherjs#1007 for details.
Tests use an unsupported compiler flag and target aspects of `unsafe` package GopherJS doesn't support.
This addresses the point of the test, but we still have to skip it because printed output is formatted slightly differently.
$callDeferred() uses `throw null` to interrupt normal execution when called from a panic(). However, it simply returns when called orderly at the end of a function. A tricky interaction may happen when one of the deferred function blocks on a channel or mutex. The goroutine will get paused and when it unblocks, deferrals will continue to execute via $callDeferred() at the end of the function. Under that circumstance `throw null` trick is unnecessary since the main body of the function is outside of the call stack anyway. This fix allows fixedbugs/issue40629.go test to pass successfully.
3f703aa enabled source map support in the CI, so this test is now able to pass.
These tests take quite a while to complete, so build able to focus on our own tests is a time saver during development.
@@ -127,6 +125,20 @@ var knownFails = map[string]failReason{ | |||
"fixedbugs/issue30977.go": {category: neverTerminates, desc: "does for { runtime.GC() }"}, | |||
"fixedbugs/issue32477.go": {category: notApplicable, desc: "uses runtime.SetFinalizer and runtime.GC"}, | |||
"fixedbugs/issue32680.go": {category: notApplicable, desc: "uses -gcflags=-d=ssa/check/on flag"}, | |||
|
|||
// These are new tests in Go 1.13-1.16. | |||
"fixedbugs/issue19113.go": {category: lowLevelRuntimeDifference, desc: "JavaScript bit shifts by negative amount don't cause an exception"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this sort of thing worth mentioning in the compat table?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this is a good idea, although I am again torn about where to put that note. docs/packages.md is supposed to be about packages and as a user I wouldn't expect to find runtime compatibility information there. On the other hand, creating a third document just for a few paragraphs' sake… Any thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe renaming packages.md into compatibility.md and putting all the bits together is a reasonable way to go, although I expect this will break a bunch of links, some of which are external, which is not great.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I think I'll do this: keep packages.md and compiler.md with their narrow-scope purpose and create a more general compatibility.md as a central doc for this kind of thing, linking out to other docs when appropriate. After a bit of thought there's merit in describing the high-level state of things, as well as low-level details like that. Since it'll deserve its own review, I'll do that in a separate PR.
This PR addresses test failures supplied by the Go repository itself. Some of them were testing features GopherJS doesn't support (primarily related to unsafe and memory management), and some were legitimate GopherJS bugs, which I fixed for the most part. See more details in commit messages.
I believe this should be the final PR for Go 1.16.2 standard library support. Phew.