Remove test-function-check_playbook_file_removed_and_added test #10982
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It was concluded this test is too costy for the benefit it brings. It takes about 10 minutes and search for a very specific and rare pattern with is also pretty easy to be caught during review.
These scripts are no longer necessary.
These files are no longer necessary.
It was only used by test_ansible_file_removed_and_added.py script.
marcusburghardt
added
enhancement
|
Start a new ephemeral environment with changes proposed in this pull request: Fedora Environment Oracle Linux 8 Environment |
|
Code Climate has analyzed commit ab8570f and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 100.0% (50% is the threshold). This pull request will bring the total coverage in the repository to 53.3% (0.0% change). View more on Code Climate. |
|
/packit test |
jan-cerny
approved these changes
Aug 11, 2023
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
The
ansible-file-removed-and-addedtest in upstream CI is a very slow test which usually takes 8 - 10 minutes, as reported by @jan-cerny. I investigated this case and concluded the test can be safely removed.Context:
The test was introduced to catch a very specific case already fixed by #7228
I believe this was one of my first PRs in the project and on it I fixed an inefficient usage of Ansible modules.
The Playbook should use the
copymodule for that specific case but instead it used an inefficient combination offileandlineinfileto achieve the same outcome.Back in the time I remember to search this pattern in other rules but this was, fortunately, the unique case I found.
So, the test was introduced by #7376 in order to catch if this specific case is again proposed in any PR.
Rationale:
It is very hard to have this specific case again in any PR. This never happened again in within at least two years.
If this happen, it is also pretty easy to see during the review of Ansible playbooks. If not, it is not also harmful. It will show unnecessary
changedin a eventual task.Review Hints:
This PR simply removes stuff. If nothing is broken by these changes, it should be fine.