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 an issue with stash #623

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

adamretter
Copy link

Summary

If you use this to clone a new repo but also have keep_local_changes => true set then the git stash pop will fail with an error. This corrects the problem.

Checklist

  • 🟢 Spec tests.
  • 🟢 Acceptance tests.
  • Manually verified. (For example puppet apply)

…nd then 'git stash pop' could be called on a fresh clone, and that caused 'git stash pop' to fail.
@adamretter adamretter requested review from bastelfreak, smortex and a team as code owners September 18, 2023 22:07
@CLAassistant
Copy link

CLAassistant commented Sep 18, 2023

CLA assistant check
All committers have signed the CLA.

lib/puppet/provider/vcsrepo/git.rb Outdated Show resolved Hide resolved
lib/puppet/provider/vcsrepo/git.rb Outdated Show resolved Hide resolved
@adamretter
Copy link
Author

@kenyon Just wondering if we could get this re-reviewed and merged?

@kenyon
Copy link

kenyon commented Oct 2, 2023

I don't work for Puppet. Someone from Puppet needs to let the tests run. I'd also suggest a more descriptive title for the pull request, since the title is used in the changelog.

# @!visibility private
def uncommitted_changes?
at_path do
git_with_identity('status', '--porcelain').empty?
Copy link
Collaborator

Choose a reason for hiding this comment

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

This change the sequence of commands run, you need to adjust CI to take this into account (see failed CI at the bottom of the "Conversation" tab).


Need guidance? Read on 😃

When reading the output from RSpec, you can identify 3 sections:

  1. The output of the various tests with some details on failures;
  2. A list of all failures details with backtraces;
  3. A list of failed tests.

3 is the most handy to locate the tests you want to adjust (direct view of files and lines), while 1 & 2 tell you what is wrong.

If you consider the first failure in the "third section", you see that the error occurred in this test:

it "executes 'git clone' and 'git checkout -b'" do
resource[:revision] = 'only/remote'
expect(Dir).to receive(:chdir).with('/').once.and_yield
expect(Dir).to receive(:chdir).with('/tmp/test').at_least(:once).and_yield
expect(provider).to receive(:exec_git).with('clone', resource.value(:source), resource.value(:path))
expect(provider).to receive(:update_submodules)
expect(provider).to receive(:update_remote_url).with('origin', resource.value(:source)).and_return false
expect(provider).to receive(:exec_git).with('branch', '--no-color', '-a').and_return(branch_a_list(resource.value(:revision)))
expect(provider).to receive(:exec_git).with('checkout', '--force', resource.value(:revision))
provider.create
end

If you scroll up in the second section, you see the actual error:

  1) Puppet::Type::Vcsrepo::ProviderGit when with an ensure of present when with an ensure of present - with a revision that is a remote branch executes 'git clone' and 'git checkout -b'
     Failure/Error: exec_git(*args)

       Vcsrepo[test](provider=git) received :exec_git with unexpected arguments
         expected: ("branch", "--no-color", "-a")
              got: ("status", "--porcelain")
     # ./lib/puppet/provider/vcsrepo/git.rb:691:in `git_with_identity'
     # ./lib/puppet/provider/vcsrepo/git.rb:510:in `block in uncommitted_changes?'
     # ./lib/puppet/provider/vcsrepo.rb:53:in `block in at_path'
     # ./lib/puppet/provider/vcsrepo.rb:52:in `at_path'
     # ./lib/puppet/provider/vcsrepo/git.rb:509:in `uncommitted_changes?'
     # ./lib/puppet/provider/vcsrepo/git.rb:426:in `checkout'
     # ./lib/puppet/provider/vcsrepo/git.rb:22:in `create'
     # ./spec/unit/puppet/provider/vcsrepo/git_spec.rb:43:in `block (4 levels) in <top (required)>'
     # ./vendor/bundle/ruby/2.7.0/bin/rspec:23:in `load'
     # ./vendor/bundle/ruby/2.7.0/bin/rspec:23:in `<top (required)>'
     # /opt/hostedtoolcache/Ruby/2.7.8/x64/bin/bundle:23:in `load'
     # /opt/hostedtoolcache/Ruby/2.7.8/x64/bin/bundle:23:in `<main>'

In this case, adding a line like the following one between lines 40 and 41 should fix the issue:

   expect(provider).to receive(:exec_git).with('status', '--porcelain').and_return('') 

Thanks!

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.

4 participants