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

ci: annotate pytest failures on GHA #2448

Merged
merged 3 commits into from
May 10, 2023
Merged

Conversation

agoose77
Copy link
Collaborator

@agoose77 agoose77 commented May 9, 2023

Closes #2405

@agoose77 agoose77 marked this pull request as ready for review May 9, 2023 13:11
@agoose77 agoose77 requested a review from jpivarski May 9, 2023 13:12
@agoose77
Copy link
Collaborator Author

agoose77 commented May 9, 2023

See test_XXXX_failure.py for a test that is deliberately failing to demonstrate this feature.

Copy link
Member

@jpivarski jpivarski left a comment

Choose a reason for hiding this comment

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

I see, although PyLint doesn't like tests that so obviously would always fail. Perhaps you can trick it like this:

import sys

assert sys.platform == "wacky"

PyLint can't know what all of the platform types are, and that this might actually run on a wacky system someday. We, however, know that it will not. (If you want to be more sure of it, generate a long random name.)

@agoose77
Copy link
Collaborator Author

I see, although PyLint doesn't like tests that so obviously would always fail.

I'm not quite following here — I added these failing tests to trigger a failed run with annotations (on GitHub)
image

Incidentally, I don't see any pylint warnings about this trivial assertion (either on GHA, or locally)

@jpivarski
Copy link
Member

Oh! This was the intention! I had thought these were PyLint failures because PyLint has always been plugged into this system (correlating errors to code in GitHub). Also, PyLint could be smart enough to find out that this assertion would always fail, and that's why I was suggesting a way to fool it. But I misunderstood what was supposed to happen here.

@agoose77
Copy link
Collaborator Author

Ah, I see! Well, if you're happy with this feature, I'll remove the failing test!

@agoose77 agoose77 force-pushed the agoose77/ci-annotate-test-failures branch from 880b780 to c13b710 Compare May 10, 2023 09:13
@agoose77 agoose77 temporarily deployed to docs-preview May 10, 2023 09:24 — with GitHub Actions Inactive
@codecov
Copy link

codecov bot commented May 10, 2023

Codecov Report

Merging #2448 (c13b710) into main (3d84e05) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

@agoose77 agoose77 merged commit 8957c24 into main May 10, 2023
@agoose77 agoose77 deleted the agoose77/ci-annotate-test-failures branch May 10, 2023 09:51
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.

Use pytest plugin to annotate failures on GHA
2 participants