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

Convert the spec for Bundler::UpdateChecker to use project based fixtures. #3370

Merged
merged 3 commits into from
Mar 26, 2021

Conversation

brrygrdn
Copy link
Contributor

@brrygrdn brrygrdn commented Mar 26, 2021

This PR extracts from #3350

It prepares the way for bundler version detection by ensuring we can readily produce builds against lockfiles with the correct BUNDLED WITH values based on test parameters.

Methodology

In order to verify the change over, I used a helper shim to force any tests that called fixture to load gemfiles to raise, e.g.

# bundler/spec/spec_helper.rb
alias non_project_fixture fixture

def fixture(*name)
  if name.any? { |folder| %w(gemfiles gemspecs lockfiles).include? folder }
    raise "Non-Project Fixture Loaded: '#{File.join(name)}'."
  end

  non_project_fixture(*name)
end

A large number of the bundler1 fixtures already existed, so it was just a case of generating any missing files, checking their naming lined up and then finally grepping the file for _fixture_name to ensure no test cases were quietly failing over to the top-level dependency files.

@brrygrdn brrygrdn requested a review from a team as a code owner March 26, 2021 16:04
@brrygrdn
Copy link
Contributor Author

Aha, I forgot I'll need to commit a stubbed version of bundler_project_dependency_files for these tests to pass, will add that now

@brrygrdn brrygrdn force-pushed the brrygrdn/update-checker-spec-proj-fixtures branch from 92e50a6 to 5689bbb Compare March 26, 2021 16:30
# Load project files prepended with the bundler version, which is currently only ever bundler1
def bundler_project_dependency_files(project)
project_dependency_files(File.join("bundler1", project))
end
Copy link
Contributor Author

@brrygrdn brrygrdn Mar 26, 2021

Choose a reason for hiding this comment

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

This method provides a forward compatible way to load any fixtures modified in this PR, I left any existing calls to project_dependency_files alone to avoid bloating the diff.

Copy link
Member

@jurre jurre left a comment

Choose a reason for hiding this comment

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

Right on 🤙

I've reviewed the changes to the tests, and based on them passing trust that the fixtures are 💯

@@ -27,6 +27,9 @@

def project_dependency_files(project)
project_path = File.expand_path(File.join("../../spec/fixtures/projects/bundler1", project))

raise "Fixture does not exist for project: '#{project}'" unless Dir.exist?(project_path)
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@feelepxyz feelepxyz left a comment

Choose a reason for hiding this comment

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

🙌

@brrygrdn brrygrdn enabled auto-merge March 26, 2021 17:19
@brrygrdn brrygrdn merged commit 9f5e4dc into main Mar 26, 2021
@brrygrdn brrygrdn deleted the brrygrdn/update-checker-spec-proj-fixtures branch March 26, 2021 17:25
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.

3 participants