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

Display test failures in the gradle check status report. #5200

Merged
merged 3 commits into from
Nov 15, 2022

Conversation

dblock
Copy link
Member

@dblock dblock commented Nov 10, 2022

Signed-off-by: Daniel (dB.) Doubrovkine dblock@amazon.com

Description

Display test failures in the gradle check status report.
I tested this by hard-coding a few failed build URIs, so hopefully this doesn't break everything, but to actually open the PR we need a pull request target to work.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@dblock dblock force-pushed the display-test-failures branch from 44a6755 to 7f2e175 Compare November 10, 2022 17:07
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@dblock dblock force-pushed the display-test-failures branch 3 times, most recently from 4c41428 to 731aa1f Compare November 10, 2022 17:21
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@dblock dblock force-pushed the display-test-failures branch from 731aa1f to 2e79652 Compare November 10, 2022 17:35
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@dblock dblock force-pushed the display-test-failures branch 2 times, most recently from 4e80ab8 to 560f26f Compare November 10, 2022 17:44
@dblock dblock marked this pull request as ready for review November 10, 2022 17:46
@dblock dblock requested review from a team and reta as code owners November 10, 2022 17:46
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@reta
Copy link
Collaborator

reta commented Nov 10, 2022

@dblock I think we could use community provided test reporters, fe [1], what do you think?

[1] https://github.com/marketplace/actions/test-reporter

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@dblock
Copy link
Member Author

dblock commented Nov 10, 2022

@dblock I think we could use community provided test reporters, fe [1], what do you think?

[1] https://github.com/marketplace/actions/test-reporter

Yeah that's a good idea. Would need to fetch the test results from gradle check to use, and this is not very reassuring:

java-junit (Experimental)
Support for [JUnit](https://junit.org/) XML is experimental - should work but it was not extensively tested. To have code annotations working properly, it's required your directory structure matches the package name. This is due to the fact Java stack traces don't contain a full path to the source file. Some heuristic was necessary to figure out the mapping between the line in the stack trace and an actual source file.

Copy link
Member

@andrross andrross left a comment

Choose a reason for hiding this comment

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

I've tested the curl / jq command and it seems to work. Is there any way to test this end-to-end without just committing it to main?

- name: Extract Test Failure
if: ${{ github.event_name == 'pull_request_target' && failure() }}
run: |
TEST_FAILURES=`curl -s "${{ env.workflow_url }}/testReport/api/json?tree=suites\[cases\[status,className,name\]\]" | jq -r '.. | objects | select(.status=="FAILED",.status=="REGRESSION") | (.className + "." + .name)' | uniq`
Copy link
Member

Choose a reason for hiding this comment

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

Maybe use uniq -c to get a count of failures?

Copy link
Member Author

@dblock dblock Nov 15, 2022

Choose a reason for hiding this comment

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

I wanted to display the actual failures instead.

Copy link
Member

Choose a reason for hiding this comment

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

-c will give you both the failure and the count:

curl -s "https://build.ci.opensearch.org/job/gradle-check/6876/testReport/api/json?tree=suites\[cases\[status,className,name\]\]" | jq -r '.. | objects | select(.status=="FAILED",.status=="REGRESSION") | (.className + "." + .name)' | uniq -c
      2 org.opensearch.index.ShardIndexingPressureConcurrentExecutionTests.testCoordinatingPrimaryThreadedUpdateToShardLimitsAndRejections
      4 org.opensearch.index.mapper.GeoPointFieldMapperTests.testLatLonInArrayMoreThanThreeValues

Copy link
Member

Choose a reason for hiding this comment

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

Actually, I'd add | sort -n -r | head -n 10 to the command as well to put the most failures at the top and have a limit so we don't spam the correspondence in the case things go badly.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added. Nice bash-fu @andrross :)

@andrross
Copy link
Member

Also I agree there are probably better ways to do this but this seems like a quick and easy improvement that can be done now

Signed-off-by: Daniel (dB.) Doubrovkine <dblock@amazon.com>
@dblock dblock force-pushed the display-test-failures branch from 560f26f to 8dc990c Compare November 15, 2022 19:56
@dblock
Copy link
Member Author

dblock commented Nov 15, 2022

@andrross This PR is just a lazy feature, you can click on the gradle check and see the test failures. So I don't feel particularly compelled to merge it, but if you think it's useful, maybe you can convince @reta of its benefits? :)

@reta
Copy link
Collaborator

reta commented Nov 15, 2022

@andrross This PR is just a lazy feature, you can click on the gradle check and see the test failures. So I don't feel particularly compelled to merge it, but if you think it's useful, maybe you can convince @reta of its benefits? :)

haha @dblock , I am absolutely not against it, just wanted to make integration of such a feature as simple as possible (using available Github actions but it seems like we need to allow external actions on repository explicitly)

@andrross
Copy link
Member

The test-reporter action seems to put the results on the github action screen (which you'd have to click to get to) and doesn't seems like it would add much value given that we have a handy link to the test results anyway. Am I misinterpreting that?

I like that this solution ends up being very in-your-face about the flaky failures and you don't have to click anything to see what failed. Might be more motivation to help drive these down :)

@reta
Copy link
Collaborator

reta commented Nov 15, 2022

The test-reporter action seems to put the results on the github action screen (which you'd have to click to get to) and doesn't seems like it would add much value given that we have a handy link to the test results anyway. Am I misinterpreting that?

I like that this solution ends up being very in-your-face about the flaky failures and you don't have to click anything to see what failed. Might be more motivation to help drive these down :)

@andrross fair enough, let's give it a try?

Signed-off-by: dblock <dblock@dblock.org>
Signed-off-by: dblock <dblock@dblock.org>
@dblock dblock requested a review from andrross November 15, 2022 20:25
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@andrross andrross merged commit 51991e0 into opensearch-project:main Nov 15, 2022
@dblock dblock added the backport 2.x Backport to 2.x branch label Nov 15, 2022
opensearch-trigger-bot bot pushed a commit that referenced this pull request Nov 15, 2022
* Display test failures in the gradle check status report.

Signed-off-by: Daniel (dB.) Doubrovkine <dblock@amazon.com>

* Add counts of failures.

Signed-off-by: dblock <dblock@dblock.org>

* Limit to 10 failures.

Signed-off-by: dblock <dblock@dblock.org>

Signed-off-by: Daniel (dB.) Doubrovkine <dblock@amazon.com>
Signed-off-by: dblock <dblock@dblock.org>
(cherry picked from commit 51991e0)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@dblock dblock mentioned this pull request Nov 15, 2022
dblock added a commit that referenced this pull request Nov 15, 2022
…t. (#5266)

* Display test failures in the gradle check status report. (#5200)

* Display test failures in the gradle check status report.

Signed-off-by: Daniel (dB.) Doubrovkine <dblock@amazon.com>

* Add counts of failures.

Signed-off-by: dblock <dblock@dblock.org>

* Limit to 10 failures.

Signed-off-by: dblock <dblock@dblock.org>

Signed-off-by: Daniel (dB.) Doubrovkine <dblock@amazon.com>
Signed-off-by: dblock <dblock@dblock.org>
(cherry picked from commit 51991e0)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* Quote failures.

Signed-off-by: dblock <dblock@dblock.org>

Signed-off-by: Daniel (dB.) Doubrovkine <dblock@amazon.com>
Signed-off-by: dblock <dblock@dblock.org>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: dblock <dblock@dblock.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Backport to 2.x branch skip-changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants