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

Sanity tests: ignore shebangs in files/ and templates/ in collection roles and integration test targets #79700

Open
wants to merge 2 commits into
base: devel
Choose a base branch
from

Conversation

felixfontein
Copy link
Contributor

SUMMARY

Fixes #76612.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

ansible-test shebang sanity test

@ansibot ansibot added affects_2.15 feature This issue/PR relates to a feature request. has_issue needs_triage Needs a first human triage before being processed. test This PR relates to tests. labels Jan 10, 2023
@ansibot

This comment was marked as resolved.

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jan 10, 2023
@MarkusTeufelberger
Copy link
Contributor

Thanks a lot! :-)

@mkrizek mkrizek removed the needs_triage Needs a first human triage before being processed. label Jan 10, 2023
@mkrizek mkrizek requested a review from mattclay January 10, 2023 15:30
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Jan 18, 2023
@MarkusTeufelberger
Copy link
Contributor

Anything still missing for a successful review?

Copy link
Member

@mattclay mattclay left a comment

Choose a reason for hiding this comment

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

I still need to look at this further, and take into consideration how it fits in with other sanity tests. If it were to be merged, it should at least be updated so the change only affects collections.

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Feb 13, 2023
@ansibot

This comment was marked as resolved.

@ansibot ansibot added ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. and removed ci_verified Changes made in this PR are causing tests to fail. needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Feb 13, 2023
@ansibot ansibot added stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. stale_review Updates were made after the last review and the last review is more than 7 days old. labels Feb 21, 2023
@MarkusTeufelberger
Copy link
Contributor

@mattclay - any further thoughts? There could of course be checks for shebangs in files/templates folders, even enforced by sanity checks, but forcing every shebang in there to be from the list in this sanity test here is imho a clear bug.

@mattclay
Copy link
Member

@MarkusTeufelberger I started looking at this, an other similar issues with sanity tests, but I haven't had time to finish. It's still on my list to get back to.

@MarkusTeufelberger
Copy link
Contributor

RC1 for 2.15 is getting closer and closer, it would be really nice to squeeze this one in.

@felixfontein
Copy link
Contributor Author

Feature freeze already passed :(

@MarkusTeufelberger
Copy link
Contributor

Well, it is a bugfix, not a feature I hope.

@mattclay
Copy link
Member

This is being handled as a feature request, not a bug fix. It will not be in 2.15.

@ansibot ansibot removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. stale_review Updates were made after the last review and the last review is more than 7 days old. labels Jun 16, 2023
@ansibot ansibot added the stale_review Updates were made after the last review and the last review is more than 7 days old. label Jul 12, 2023
@felixfontein
Copy link
Contributor Author

@mattclay any chance for 2.16?

@ohdearaugustin
Copy link

This would be great. The change is not that big and would be nice to make into the next release.

@MarkusTeufelberger
Copy link
Contributor

Still an issue on 2.18 devel.

@ansibot ansibot added the needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. label Apr 9, 2024
@webknjaz

This comment was marked as resolved.

This comment was marked as resolved.

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Apr 9, 2024
@webknjaz
Copy link
Member

The branch needs rebasing.

@webknjaz webknjaz added the ci_verified Changes made in this PR are causing tests to fail. label Apr 10, 2024
felixfontein and others added 2 commits April 10, 2024 18:20
@ansibot ansibot removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. ci_verified Changes made in this PR are causing tests to fail. labels Apr 10, 2024
@felixfontein
Copy link
Contributor Author

@webknjaz done.

@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Apr 24, 2024
@felixfontein
Copy link
Contributor Author

@webknjaz @mattclay any chance of getting this (or something equivalent) merged?

@MarkusTeufelberger
Copy link
Contributor

Please add this to 2.18!

@felixfontein
Copy link
Contributor Author

Considering https://forum.ansible.com/t/5719/4 I doubt that anything will happen here :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects_2.15 feature This issue/PR relates to a feature request. has_issue stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. stale_review Updates were made after the last review and the last review is more than 7 days old. test This PR relates to tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Shebang code-smell sanity test triggers on files in roles
7 participants