Skip to content

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

Merged
merged 14 commits into from
Apr 4, 2021

Conversation

nevkontakte
Copy link
Member

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.

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.
@nevkontakte nevkontakte requested a review from flimzy March 29, 2021 08:54
@@ -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"},
Copy link
Member

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?

Copy link
Member Author

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?

Copy link
Member Author

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.

Copy link
Member Author

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.

@nevkontakte nevkontakte merged commit 11747e9 into gopherjs:go1.16-stdlib Apr 4, 2021
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