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

Fix issue where test leaves a dirty git tree #23231

Merged
merged 1 commit into from
Oct 14, 2024

Conversation

Fryguy
Copy link
Member

@Fryguy Fryguy commented Oct 14, 2024

In 195a04f, a test was removed that had created a local repo in the git_repos dir, but the cleanup from the after block was not removed. This cleanup removes the .gitkeep file, which causes a dirty tree.

@kbrock Please review.

In 195a04f, a test was removed that had created a local repo in the
git_repos dir, but the cleanup from the after block was not removed.
This cleanup removes the .gitkeep file, which causes a dirty tree.
@Fryguy Fryguy force-pushed the fix_test_contamination branch from 0a0485b to a4da531 Compare October 14, 2024 17:10
@Fryguy
Copy link
Member Author

Fryguy commented Oct 14, 2024

@agrare I noticed you had opened #22960, but it was WIP. I think this does the same thing?

#22960 sort of does multiple things...this change, which causes a dirty repository, but then also the git_repository_spec leaves behind dirty directories. I only did the former here. I'm good with merging #22960 if you unWIP it

@miq-bot
Copy link
Member

miq-bot commented Oct 14, 2024

Checked commit Fryguy@a4da531 with ruby 3.1.5, rubocop 1.56.3, haml-lint 0.51.0, and yamllint
1 file checked, 0 offenses detected
Everything looks fine. 👍

@agrare
Copy link
Member

agrare commented Oct 14, 2024

Hm it was a while ago but I thought I had to do more than just delete this in order to not leave lockfiles around after the tests ran, let me double check...

@Fryguy
Copy link
Member Author

Fryguy commented Oct 14, 2024

Specifically for the dirty tree from embedded_ansible_spec, you only this need change. If you want to fully cleanup the git_repos dir after the git_repository_spec you need the other change.

@agrare
Copy link
Member

agrare commented Oct 14, 2024

Yeah after running these tests I still have

$ ls data/git_repos/locks/ -a
.              2000000000009   54000000000004  54000000000013  61000000000246  61000000000263  61000000000278  65000000000306  65000000000318  65000000000327  65000000000339
..             2000000000015   54000000000005  54000000000014  61000000000247  61000000000264  61000000000279  65000000000307  65000000000319  65000000000328  65000000000340
2000000000002  2000000000016   54000000000006  54000000000016  61000000000248  61000000000266  61000000000280  65000000000308  65000000000320  65000000000329  65000000000341
2000000000003  2000000000017   54000000000007  54000000000019  61000000000249  61000000000267  61000000000281  65000000000311  65000000000321  65000000000330  65000000000342
2000000000004  2000000000018   54000000000008  54000000000020  61000000000258  61000000000268  61000000000282  65000000000313  65000000000322  65000000000332  65000000000343
2000000000005  2000000000019   54000000000009  54000000000021  61000000000259  61000000000269  61000000000283  65000000000314  65000000000323  65000000000334  65000000000344
2000000000006  54000000000001  54000000000010  54000000000022  61000000000260  61000000000270  65000000000303  65000000000315  65000000000324  65000000000336  65000000000345
2000000000007  54000000000002  54000000000011  61000000000244  61000000000261  61000000000271  65000000000304  65000000000316  65000000000325  65000000000337  65000000000346
2000000000008  54000000000003  54000000000012  61000000000245  61000000000262  61000000000277  65000000000305  65000000000317  65000000000326  65000000000338  .gitkeep

I suppose those aren't hurting anything other than cluttering up the locks dir, we can merge this then I'll rebase my PR and fix just the locks being left on the filesystem.

@agrare agrare assigned agrare and unassigned kbrock Oct 14, 2024
@agrare agrare merged commit 1a3ceee into ManageIQ:master Oct 14, 2024
12 checks passed
@Fryguy Fryguy deleted the fix_test_contamination branch October 14, 2024 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants