LUCENE-9077 Print repro line for failed tests

[madrob]

Description

Print the 'reproduce with' line for failed tests

Tests

I did not test this with multiple tests running in parallel yet because I'm still figuring out all the right ways to do Gradle things.

Checklist

Please review the following and check all that apply:

• I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
• I have created a Jira issue and added the issue ID to my pull request title.
• I have given Solr maintainers access to contribute to my PR branch. (optional but recommended)
• I have developed this patch against the master branch.
• I have run ant precommit and the appropriate test suite.
• I have added tests for my changes.
• I have added documentation for the Ref Guide (for Solr changes only).

[dweiss]

Hi Mike. I looked at gradle's internal APIs again and I don't think we'll be able to dodge having a custom test output listener. I would rather have it complete two goals at once: emit test failure information and dump test output on failure.

This has been done by Elasticsearch under this issue: elastic/elasticsearch#31496

The discussion there is enlightening about certain aspects of handling gradle's output. We can also borrow some of the code (like ErrorReportingTestListener.java that buffers up test output and discards it if the test passes). I believe some changes would have to be implemented -- Solr tests emit TONS of output and it will lead to OOM exceptions of the build system... It should spill to disk after it reaches a certain threshold. But even the in-memory version would be a start.

Would you be willing to bite into this (borrowing some code from ES)?

[uschindler]

By the way: is security manager working correctly in Gradle? Because the elasticsearch issue mentions that security manager is not supported with the Gradle runner.

Or is this solved already?

[dweiss]

Yes, it works. Low level jvm messages or debugging security, network, etc, screw it up badly though. But tests work.

[madrob]

I have a rudimentary in memory version now, can look at the disk spilling version later. Ended up fighting with Gradle more than I thought I would need to get even this far, although the disk spilling version should be straightforward from here. LMK what you think.

[dweiss]

Thanks. I'll try to combine what you did with what folks at ES did and have something workable. It is important to get it right as people will rely on it.

Ended up fighting with Gradle more than I thought I would need to get even this far

Heh. I warned about this a long time ago. :) Gradle is fun but when something doesn't work as expected it can also turn into a hairy ball of complicated mess.

[madrob]

Stepping back, I'm a little confused what the motivation for the requirement to print test output on failure comes from. We already have the HTML/XML reports that include it, users should be able to view those (gradle output tells you where they are on local disk) and CI can archive and save those.

If we want to print the output, I think we should be leveraging these already created reports instead of trying to figure out our own thing?

[madrob]

I was able to go back and take another look at what ES had and it turned out to be pretty straightforward to plug in to what we have.

[dweiss]

Hi Mike. I have a version working too -- seems like we've been working on this concurrently. Let me polish the remaining bit and I'll post it here (I changed certain aspects of ES's implementation).

[dweiss]

I'm a little confused what the motivation for the requirement to print test output on failure comes from.

There are a few reasons. Off the top of my head:

1. HTML report in Gradle is quite terrible and XML reports have split sysout/syserr outputs.

2. buildscan reports would be a nicer alternative but Solr emits TONS of logs. Literally megabytes.

There is also an issue of running ant ant gradle in parallel on master. We can't afford to just switch over to gradle in one go -- we will have to live with both systems at the same time, at least for some time.

[dweiss]

I just pushed a version which has the output handler precompiled as Java code (faster than groovy) and does output spilling into task's temporary folder.

14dd5a5

I think it's working correctly but feel free to eyeball and test!

[madrob]

I left a few comments on the file there, but I'm not sure how easy those are to go fetch for you. A few questions about the architecture:

1. Are we buffering output in both PrefixWriter and SpillWriter? Why do we need to do that in two places?
2. No special handling for the reproduce with line?

[dweiss]

Are we buffering output in both PrefixWriter and SpillWriter? Why do we need to do that in two places?

To distinguish between syserr and sysout; they are flushed independently if they're mixed (with a different line prefix). This helps to distinguish what was emitted from where. So you'd get consistent output even if you used print() (without a flush).

  1> sysout print
  2> syserr print

Arguably this could be simplified to just use a single stream but found it really useful to distinguish between syserr and sysout in many debugging sessions...

bq. No special handling for the reproduce with line?

One thing at a time. Reproduce line has to work for both ant and gradle. Also - Gradle failed test summary could display a much better (shorter) version of the reproduce line. It knows which properties have been changed from their default values and what the test seed was. It only needs to display something like:

Reproduce with: ./gradlew -p [project] test --tests [testcase] [non-default properties]

the internal reproduce line is still useful as a "verbose" information about which properties were used but I think this can be an additional thing to the above.

I'll work on it later today (since I know where all the bits and pieces are). This said, I appreciate you getting familiar with all of this infrastructure, Mike. Please keep doing this!

[madrob]

Thanks, Dawid! I'll close the PR and let you take it from here then!