Add Go repository compiler tests, triage known failures. #296
Conversation
|
So, one thing to figure out, the current output is very verbose. With I've made it so that If we're going to actively and quickly fix all the issues, maybe keeping verbose output is okay. |
|
I'll push a new commit with non-verbose output for CI; let me know which you like better. |
|
Okay, compare overly-verbose output (with full error output for known failures), previously: https://circleci.com/gh/gopherjs/gopherjs/564 Versus the updated verbose output (just a list of tests and their statuses): https://circleci.com/gh/gopherjs/gopherjs/565 I think the new version is much better. If you want to run the test runner manually and see full error details for known failures, just use This way, only deviations in status (previously succeeding tests failing, known failures suddenly succeeding, etc.) will print their full details. Otherwise it's just 1 line per test run, which seems reasonable for |
9f95a69 to
bc81548
Compare
|
Everything here is ready from my side. I think after you review it @neelance, it may be better to squash into 1 commit, then merge. |
|
|
||
| goroutine 1 [running]: | ||
| github.com/gopherjs/gopherjs/compiler.(*funcContext).fixNumber(0xc820164210, 0xc82000f360, 0x9088a0, 0xc820018ea0) | ||
| /Users/Dmitri/Dropbox/Work/2013/GoLand/src/github.com/gopherjs/gopherjs/compiler/expressions.go:1176 +0x235 |
There was a problem hiding this comment.
Yes, but it's just a description of the error. I copy pasted the first line/few lines of the panic output here.
Ideally, these descriptions can be replaced with more accurate ones if someone goes through them and triages the issues carefully.
There was a problem hiding this comment.
Fixed in 44bb9b5 anyway.
There was a problem hiding this comment.
Let's at least make them a bit more pretty by making the paths relative to project root and removing all those pointer values.
There was a problem hiding this comment.
Whoops, didn't see this comment. I agree, let's make the descriptions nicer in a followup commit.
Although I find it I end up just trying to investigate and fix the bug if possible right away... E.g., #298.
|
LGTM |
Add a modified version of go/test/run.go for GopherJS. The run.go file should be run via native Go, but it will itself invoke `gopherjs run` for running tests. All modifications are marked with a "// GOPHERJS" comment. At this time, only a limited subset of folders and test types is supported. More can be added. Within the tests that are run, there are currently 234 tests passing and 34 failing. The failing test are marked as known failures and do not trigger CI failure. As the bugs are resolved, the knownFails map in run.go should be updated to remove resolved failures. The test will currently fail if any of the tests that were expected to fail unexpectedly pass (if it's not an accident, then knownFails map should be updated accordingly). I hope this will not cause too much noise; it can be changed if so. Four of the tests are compiler panics, they should probably be fixed first. The remaining ones are briefly described but not triaged further than that. The run.go test runner can be run manually via: go run run.go -v -summary Or to include known fails full error messages: go run run.go -v -summary -show_known_fails Add fixedbugs/issue10607.go to known failures. It cannot run because it uses os/exec. This test is meaningless anyway because it tests native go compiler linking modes. Remove unneeded TODOs. Don't output details of known test failures unless show_known_fails flag is on. This makes normal verbose output much more readable and suitable for CI. Change TestGoRepositoryCompilerTests to stream output as it runs. Previously, it would only display all of output when it finished. Fix issue where specifying specific test to run failed to run it. It was because filepath.Split left a trailing slash after dir, so "fixedbugs/" != "fixedbugs". Use filepath.Clean on input t.dir to resolve this. Always display known fails details if -show_known_fails flag is on (even without -v). Previously, both -v and -show_known_fails had to be on. Also tweak the reproduce command printed to include -show_known_fails flag, since it's needed. Remove absolute paths from known fails descriptions.
44bb9b5 to
587ec90
Compare
|
Squashed into 1 commit, merging. |
Add a modified version of go/test/run.go for GopherJS. The run.go file should be run via native Go, but it will itself invoke
gopherjs runfor running tests. All modifications are marked with a"// GOPHERJS"comment.At this time, only a limited subset of folders and test types is supported. More can be added.
Within the tests that are run, there are currently 234 tests passing and 34 failing. The failing test are marked as known failures and do not trigger CI failure.
As the bugs are resolved, the knownFails map in run.go should be updated to remove resolved failures. The test will currently fail if any of the tests that were expected to fail unexpectedly pass (if it's not an accident, then knownFails map should be updated accordingly). I hope this will not cause too much noise; it can be changed if so.
Four of the tests are compiler panics, they should probably be fixed first. The remaining ones are briefly described but not triaged further than that.
The
run.gotest runner can be run manually via:Or to include known fails full error messages: