Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Rewrite to take test2json input #9

Merged
merged 1 commit into from
Sep 20, 2018
Merged

Rewrite to take test2json input #9

merged 1 commit into from
Sep 20, 2018

Conversation

tbg
Copy link
Member

@tbg tbg commented 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.

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 tbg requested a review from benesch September 19, 2018 14:43
tbg added a commit to tbg/cockroach that referenced this pull request Sep 19, 2018
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
Copy link

@benesch benesch left a comment

Choose a reason for hiding this comment

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

I only skimmed this, but the part where we can conditionally turn this on in an upstream PR by passing -json seems great. Don't think you need to wait for a more thorough review.

@tbg tbg merged commit b1ed8e6 into cockroachdb:master Sep 20, 2018
@tbg
Copy link
Member Author

tbg commented Sep 20, 2018

TFTR!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants