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

Remove deletion of data/git_repos for EmbeddedAnsible tests #22960

Merged
merged 1 commit into from
Oct 14, 2024

Conversation

agrare
Copy link
Member

@agrare agrare commented Mar 27, 2024

The data/git_repos directory doesn't appear to be used but it does leave local git changes when running the embedded_ansible_spec.rb

$ git status

Changes not staged for commit:
  (use "git add/rm <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	deleted:    data/git_repos/locks/.gitkeep

@kbrock
Copy link
Member

kbrock commented Mar 28, 2024

I'm very happy for this change.
This frustrated me but I hadn't tracked it down.

Do we need to clean these up in another way? Like deleting all files underneath that directory (may cause issues for local development though)

@agrare
Copy link
Member Author

agrare commented Mar 28, 2024

Do we need to clean these up in another way? Like deleting all files underneath that directory (may cause issues for local development though)

Yeah I thought I'd have to delete all dirs+files under this directory, but after removing this line and running this spec file I didn't see any files in this directory left over, and and I don't see anything in the embedded_ansible seed that does anything with git_repos.

If I run the full suite I do see leftover locks, so I think other specs need to clean up their own locks and this was hiding that.

@agrare
Copy link
Member Author

agrare commented Mar 28, 2024

Okay it looks like leftover locks are from these two spec files (not surprisingly), spec/models/git_repository_spec.rb and spec/models/manageiq/providers/embedded_ansible/automation_manager/configuration_script_source_spec.rb.

@agrare
Copy link
Member Author

agrare commented Mar 28, 2024

Does anything ever delete these git_repository lockfiles at runtime?

@agrare agrare force-pushed the fix_data_git_repos_locks_deleted branch from fafbe9f to 6fa3221 Compare March 28, 2024 19:08
@agrare agrare changed the title Remove deletion of data/git_repos for EmbeddedAnsible tests [WIP] Remove deletion of data/git_repos for EmbeddedAnsible tests Apr 5, 2024
@agrare
Copy link
Member Author

agrare commented Apr 5, 2024

WIP until I cover the other specs that leave these lockfiles around

@miq-bot miq-bot added the wip label Apr 5, 2024
@miq-bot miq-bot added the stale label Jul 8, 2024
@miq-bot
Copy link
Member

miq-bot commented Jul 8, 2024

This pull request has been automatically marked as stale because it has not been updated for at least 3 months.

If these changes are still valid, please remove the stale label, make any changes requested by reviewers (if any), and ensure that this issue is being looked at by the assigned/reviewer(s).

1 similar comment
@miq-bot
Copy link
Member

miq-bot commented Oct 14, 2024

This pull request has been automatically marked as stale because it has not been updated for at least 3 months.

If these changes are still valid, please remove the stale label, make any changes requested by reviewers (if any), and ensure that this issue is being looked at by the assigned/reviewer(s).

@agrare agrare force-pushed the fix_data_git_repos_locks_deleted branch 2 times, most recently from dff35d3 to 01fcc7b Compare October 14, 2024 18:04
@agrare agrare force-pushed the fix_data_git_repos_locks_deleted branch from 01fcc7b to 4bedce6 Compare October 14, 2024 18:32
@miq-bot
Copy link
Member

miq-bot commented Oct 14, 2024

Checked commit agrare@4bedce6 with ruby 3.1.5, rubocop 1.56.3, haml-lint 0.51.0, and yamllint
2 files checked, 0 offenses detected
Everything looks fine. 🏆

@@ -26,6 +27,7 @@

GitRepository
stub_const("GitRepository::GIT_REPO_DIRECTORY", repo_dir)
stub_const("GitRepository::LOCKFILE_DIR", locks_dir)
Copy link
Member Author

Choose a reason for hiding this comment

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

NOTE unlike the git_repository_spec there isn't always a repos instance available (this also tests creating via the create_in_provider_queue method for example), since this already stubs the GIT_REPO_DIRECTORY so that it can be cleaned up later this felt like another case of the same issue.

@agrare agrare changed the title [WIP] Remove deletion of data/git_repos for EmbeddedAnsible tests Remove deletion of data/git_repos for EmbeddedAnsible tests Oct 14, 2024
@agrare
Copy link
Member Author

agrare commented Oct 14, 2024

After running the full suite no lockfiles are left over:

$ rake parallel:spec
...
Randomized with seed 19268

12171 examples, 0 failures, 4 pendings
Took 46 seconds

$ ls -la data/git_repos/locks/
total 2
drwxr-xr-x 2 adam grare 3 Oct 14 14:32 .
drwxr-xr-x 3 adam grare 3 Oct 12 17:10 ..
-rw-r--r-- 1 adam grare 0 Oct 12 17:19 .gitkeep

@agrare
Copy link
Member Author

agrare commented Oct 14, 2024

@Fryguy rebased and taken out of WIP, PTAL

@agrare
Copy link
Member Author

agrare commented Oct 14, 2024

@kbrock fyi another fix_auth sporadic failure


  1) FixAuth::AuthModel#miq_database uses random numbers for invalid
     Failure/Error: expect(ManageIQ::Password.decrypt(bad.session_secret_token)).to_not eq "newpass"

     ManageIQ::Password::PasswordError:
       cannot decrypt encrypted string
     # ./spec/tools/fix_auth/auth_model_spec.rb:118:in `block (3 levels) in <top (required)>'
     # ------------------
     # --- Caused by: ---
     # OpenSSL::Cipher::CipherError:
     #   bad decrypt
     #   ./spec/tools/fix_auth/auth_model_spec.rb:118:in `block (3 levels) in <top (required)>'

rspec ./spec/tools/fix_auth/auth_model_spec.rb:115 # FixAuth::AuthModel#miq_database uses random numbers for invalid

https://github.com/ManageIQ/manageiq/actions/runs/11333113423/job/31516627078?pr=22960#step:8:300

@agrare agrare closed this Oct 14, 2024
@agrare agrare reopened this Oct 14, 2024
@Fryguy Fryguy merged commit 1461bd7 into ManageIQ:master Oct 14, 2024
21 of 22 checks passed
@agrare agrare deleted the fix_data_git_repos_locks_deleted branch October 14, 2024 22:03
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