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

build: show specifics of test failures in github checks #516

Closed
wants to merge 7 commits into from

Conversation

keturn
Copy link
Contributor

@keturn keturn commented Sep 14, 2022

This uses https://github.com/marketplace/actions/junit-report-action so that the specifics of failed tests will be more visible in the GitHub UI, instead of just "process completed with exit code 1."

@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint.

@keturn
Copy link
Contributor Author

keturn commented Sep 14, 2022

Hmm. It successfully uploaded the junit.xml written by pytest. But I don't see any sign that it ran the test_report action.

I don't really know how GitHub Actions works. Do I need to organize the yaml differently? Or do the project's owners need to authorize the use of those third-party Actions?

@patrickvonplaten
Copy link
Contributor

@anton-l could you take a look here?

@anton-l
Copy link
Member

anton-l commented Sep 27, 2022

Hi @keturn, sorry for the late response! We have custom detailed test reports in the GH actions artifacts now: https://github.com/huggingface/diffusers/actions/runs/3100888450
Would those solve your issue?

@keturn
Copy link
Contributor Author

keturn commented Sep 27, 2022

The test report artifacts are a means to an end. The goal is to have the reports formatted in a convenient way on the build or the PR.

either as inline annotations, as in this example from junit-report-action:

annotated.png

or some kind of tabular or drill-down format as we'd see with buildbot or jenkins.

Is there any such thing currently? I didn't see any on the run you linked, but I'm not sure if that's because I was looking in the wrong place or maybe there were just no failures on that run to report.

@patrickvonplaten
Copy link
Contributor

Currently we only see easily what test failed: https://github.com/huggingface/diffusers/actions/runs/3100888450/jobs/5021661934#step:7:926 but indeed we don't have any specifics. I'd very much welcome a PR that makes this user experience nicer! I currently don't have the time to dig deeper into what exists though.

@keturn keturn marked this pull request as draft October 3, 2022 21:40
@keturn
Copy link
Contributor Author

keturn commented Oct 3, 2022

This likely depends on what settings you have for which actions may run: https://github.com/huggingface/diffusers/settings/actions

It depends on

In addition, it needs to be running with a token with write permission to make the annotations, which is why it has a separate action instead of being included as additional steps in the pr_tests action. https://docs.github.com/en/actions/security-guides/automatic-token-authentication#modifying-the-permissions-for-the-github_token

(and could also explain why it doesn't run when I push to this PR, because presumably I can't just make PRs that run arbitrary code with repository write access.)

@keturn keturn marked this pull request as ready for review October 3, 2022 22:18
build: specify which workflows trigger test_report
This is the recommendation from action-junit-report.
@patrickvonplaten
Copy link
Contributor

@anton-l I let you handle this one :-)

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the contributing guidelines are likely to be ignored.

@github-actions github-actions bot added the stale Issues that haven't received updates label Oct 28, 2022
@keturn
Copy link
Contributor Author

keturn commented Oct 28, 2022

I think this is still actionable, stale-bot.

@anton-l anton-l removed the stale Issues that haven't received updates label Oct 29, 2022
@anton-l
Copy link
Member

anton-l commented Oct 29, 2022

@keturn looks like pytest annotations aren't as inline as java ones, just a list of errors. Example from another repo, as github disables the annotation API for PRs from forks: https://github.com/agdsn/pycroft/actions/runs/3138025002/jobs/5096982313
PR annotation issue: mikepenz/action-junit-report#23

Another alternative could be this plugin, but I haven't looked into what it does under the hood yet: https://github.com/utgwkk/pytest-github-actions-annotate-failures

@keturn
Copy link
Contributor Author

keturn commented Oct 29, 2022

an HTML-formatted list of test names and their failure messages (with a clear link to it and not buried in a thousand lines of build output) is pretty much what I was going for.

though the format as shown in that pycroft example is a terrible way to present that information, as annotations all on line 1 of something that's not actually a file. Each failure is in its own box 200px tall to convey a single line of information. 😩

@anton-l
Copy link
Member

anton-l commented Oct 29, 2022

Exactly! That pytest plugin that I've linked actually formats the errors inline, but I wonder if it can annotate PR tests too. Seems that github doesn't like API calls from workflows triggered by a fork for security reasons, and the JUnit reporter is limited by that.

@patrickvonplaten
Copy link
Contributor

Closing this for now as I think the testing as is currently is good enough. Please leave a comment if you disagree.

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the contributing guidelines are likely to be ignored.

@github-actions github-actions bot added the stale Issues that haven't received updates label Dec 10, 2022
@github-actions github-actions bot closed this Dec 19, 2022
PhaneeshB pushed a commit to nod-ai/diffusers that referenced this pull request Mar 1, 2023
* Update README.md

* Create profiling_with_iree.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Issues that haven't received updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants