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

[ignoreme] experiment with go test -json #30406

Closed
wants to merge 2 commits into from

Conversation

tbg
Copy link
Member

@tbg tbg commented Sep 19, 2018

@tbg tbg requested a review from a team September 19, 2018 12:04
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@tbg tbg force-pushed the testing/failsuite branch from 0f70c00 to 10c7514 Compare September 19, 2018 12:18
tbg added a commit to tbg/go-test-teamcity that referenced this pull request Sep 19, 2018
This executes on the plan outlined in:

https://github.com/cockroachdb/go-test-teamcity/issues/5#issuecomment-422494665

by making go-test-teamcity take test2json input. More precisely, and
unfortunately, it realistically has to be taking the input of

    make test TESTFLAGS='-v -json'

which isn't pure JSON but contains a bunch of `make` and `go build`
output (requiring a bit of massaging to pluck the relevant JSON from
the input stream).

I retained the old functionality and introduced a new `-json` flag
that triggers the new code. This important because our CI scripts
are versioned, and so we need the updated code here to work with
the non-updated CI pipeline.

I initially tried to jsonify the old test cases, but they're basically
garbage and don't correspond to real test output in some cases. Instead,
I generated a new corpus from actual CI runs in the main repo from which
I copied the artifacts file:

cockroachdb/cockroach#30406

I think it's worth checking in that PR in suitable form because it'll
allow us to iterate on this parser and extend the corpus much more
easily in the future.

Two caveats:

1. the code isn't creating TeamCity Test Suite tags. I was never married
to those and have found it unfortunate that they mess up TeamCity's test
duration counting (as far as I can remember). Adding them is probably
possible, but with parallel tests (which we don't use heavily) and such
I'm pretty sure that any naive implementation would be incorrect.
2. Tests which aren't explicitly ended (due to premature exit/crash of
the package test binary) are now output at the very end of the log.
Since the input contains *all tests*, this can be a little unintuitive.
However, these final tests contain basically no information anyway, so
that seems fine. This could be improved by flushing out the tests
whenever a package-level PASS/FAIL event is encountered, but I'm not
sure it's worth doing so.

The next step, assuming the code is in good order, is to merge it and
update the CI pipeline in the main repo to pass the `-json` flags both
to `go test` and `go-test-teamcity`.
tbg added 2 commits September 19, 2018 16:48
This package serves to make it easy to improve test coverage for
go-test-teamcity in the future as problems arise, by opening a PR that
edits the CI scripts to include the COCKROACH_FAILSUITE environment
variable, triggering various interesting test failures and data races in
these newly introduced packages. The created artifacts can then be
incorporated into the test suite for go-test-teamcity (as was already
done in cockroachdb/go-test-teamcity#9).

Release note: None
@tbg tbg force-pushed the testing/failsuite branch from 3baa3d9 to 3dd0b69 Compare September 19, 2018 14:50
@tbg tbg added the X-noremind Bots won't notify about PRs with X-noremind label Jun 19, 2019
@jordanlewis
Copy link
Member

@tbg this is dead right?

@tbg tbg closed this Jan 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
X-noremind Bots won't notify about PRs with X-noremind
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants