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

Add GitHub Action to report test failures in the workflow summary #6076

Merged
merged 1 commit into from
May 10, 2024

Conversation

jamesnetherton
Copy link
Contributor

Adds a custom action to report test failures into the workflow summary page.

I added a commit that will cause some failures so it can be tested. I'll drop it before merging.

@jamesnetherton
Copy link
Contributor Author

Test failures are listed here:

https://github.com/apache/camel-quarkus/actions/runs/8997456627.

@zhfeng
Copy link
Contributor

zhfeng commented May 8, 2024

@jamesnetherton Great work!

Just wonder if it is possible to add such a comment in PR automatically?

@jamesnetherton
Copy link
Contributor Author

Just wonder if it is possible to add such a comment in PR automatically?

Yeah, I did think about that. Not sure if we'd maybe run into permissions issues (needs checking)?

I also didn't want to force the PR comment requirement, since on the nightly workflows, there is no PR.

I can try to investigate it.

@jamesnetherton
Copy link
Contributor Author

Just wonder if it is possible to add such a comment in PR automatically?

Another reason I didn't do it, is that it makes things a lot more complicated.

For it to work reliably and avoid concurrency issues, you need to capture all of the test reports from each build job. Then at the end of the build, recover them and parse out the detail.

Which is why I'm thinking of merging the current implementation and improving it later.

@jamesnetherton jamesnetherton marked this pull request as ready for review May 8, 2024 10:42
@jamesnetherton jamesnetherton marked this pull request as draft May 8, 2024 10:42
@zhfeng
Copy link
Contributor

zhfeng commented May 8, 2024

Yeah, I understand. What I need currently is just a comment with a link to the summary when there is a test failure. This could be much easier?

@jamesnetherton
Copy link
Contributor Author

This could be much easier?

Yes & no 🙂

I could try to add something. I have a feeling that it wont work with PRs from forked repositories though. It'd need a bot user token.

@zhfeng
Copy link
Contributor

zhfeng commented May 8, 2024

Hmm, no, that's definely a problem. :(

@jamesnetherton
Copy link
Contributor Author

How should I proceed?

Merge as-is and try to follow up later with improvements? Or rethink things entirely?

Even something as simple as a comment with a link is non-trivial to manage. They get stale, so need updating on future workflow runs. Or the tests pass and they become irrelevant so need minimising or deleting.

I deliberately kept the initial scope of this to be as simple as possible. It's two clicks from the PR to get to the summary. So not a massive overhead (IMO).

@zhfeng
Copy link
Contributor

zhfeng commented May 9, 2024

I'd say let's merge and keep to improve it. Thanks @jamesnetherton !

@jamesnetherton jamesnetherton marked this pull request as ready for review May 10, 2024 10:46
@jamesnetherton jamesnetherton merged commit a7b0865 into apache:main May 10, 2024
24 checks passed
@jamesnetherton jamesnetherton deleted the ci-test-report branch May 10, 2024 12:11
@jamesnetherton jamesnetherton restored the ci-test-report branch May 13, 2024 06:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants