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

Make poetry export tests optional #9887

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Secrus
Copy link
Member

@Secrus Secrus commented Dec 4, 2024

Let's make poetry-plugin-export tests optional in CI. Most changes we make won't affect the plugin, and for situations where there is a risk, we can check by applying a label and running extra pipelines.

@radoering
Copy link
Member

I do not have a strong opinion, just some thoughts:

  1. It is not easy to know if a change will affect the export plugin. When I remember my PRs that let the plugin tests fail, I probably did not expect an influence on the export plugin for the majority of them; in other words, I would not have applied the label. Thus, making the tests optional, will result in failures caused by previous, already merged changes, where we did not apply the label.
  2. I have already wondered whether we want to keep the export tests after 2.0, with the export plugin not being installed per default anymore. Why executing export tests but not bundle tests (or shell tests)? Probably, because the export plugin is more widely used and thereby more important.

Making the tests optional might be a good compromise.

@@ -75,7 +75,7 @@ jobs:
pytest-export:
name: pytest (poetry-plugin-export)
runs-on: ${{ inputs.runner }}
if: inputs.run-pytest-export
if: ${{ inputs.run-pytest-export && contains(github.event.pull_request.labels.*.name, 'test poetry export')}}
Copy link
Member

Choose a reason for hiding this comment

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

The name of the label is not consistent with our naming scheme (no spaces, but slashes). Maybe, just test/export?

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.

2 participants