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

Housekeeping #230

Closed
wants to merge 26 commits into from
Closed

Conversation

n-rodriguez
Copy link
Contributor

@n-rodriguez n-rodriguez commented Aug 20, 2024

Hi there! This PR improves various things :

Note: this PR is based on #229 so you will need to merge #229 it first

Thank you!

warn 'Your current version of Bundler does not support parallel installation. Please ' +
'upgrade Bundler to version >= 1.4.0, or invoke `appraisal` without `--jobs` option.'
warn "Your current version of Bundler does not support parallel installation. Please " +
"upgrade Bundler to version >= 1.4.0, or invoke `appraisal` without `--jobs` option."
Copy link

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [97/80]

@@ -159,8 +161,8 @@ def bundle_options(options)
if Utils.support_parallel_installation?
options_strings << "--jobs=#{jobs}"
else
warn 'Your current version of Bundler does not support parallel installation. Please ' +
'upgrade Bundler to version >= 1.4.0, or invoke `appraisal` without `--jobs` option.'
warn "Your current version of Bundler does not support parallel installation. Please " +
Copy link

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [98/80]

file.write lockfile_content.gsub(
/ #{current_directory}/,
" #{relative_path}",
" #{relative_path}"
Copy link

Choose a reason for hiding this comment

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

Style/TrailingCommaInArguments: Put a comma after the last parameter of a multiline method call.

@@ -134,8 +136,7 @@ def dependencies_entry_for_dup
@dependencies.for_dup
end

%i[gits paths platforms groups source_blocks install_if].
each do |method_name|
%i[gits paths platforms groups source_blocks install_if].each do |method_name|
Copy link

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [82/80]

require "appraisal/dependency_list"

module Appraisal
class BundlerDSL
attr_reader :dependencies

PARTS = %w(source ruby_version gits paths dependencies groups
platforms source_blocks install_if gemspec)
PARTS = %w[source ruby_version gits paths dependencies groups
Copy link

Choose a reason for hiding this comment

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

Style/MutableConstant: Freeze mutable objects assigned to constants.

@@ -25,7 +27,7 @@ def self.customize(heading, gemfile)
lockfile: "#{gemfile.send('gemfile_name')}.lock",
lockfile_path: gemfile.send("lockfile_path"),
relative_gemfile_path: gemfile.relative_gemfile_path,
relative_lockfile_path: "#{gemfile.relative_gemfile_path}.lock",
relative_lockfile_path: "#{gemfile.relative_gemfile_path}.lock"
Copy link

Choose a reason for hiding this comment

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

Style/TrailingCommaInArguments: Put a comma after the last parameter of a multiline method call.

@@ -22,7 +24,7 @@ def for_dup

def exported_options
@options.merge(
:path => Utils.prefix_path(@options[:path])
path: Utils.prefix_path(@options[:path])
Copy link

Choose a reason for hiding this comment

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

Style/TrailingCommaInArguments: Put a comma after the last parameter of a multiline method call.

module Appraisal
# Contains methods for various operations
module Utils
def self.support_parallel_installation?
Gem::Version.create(Bundler::VERSION) >= Gem::Version.create('1.4.0.pre.1')
Gem::Version.create(Bundler::VERSION) >= Gem::Version.create("1.4.0.pre.1")
Copy link

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [81/80]


expect(content_of 'gemfiles/ruby_version.gemfile').to eq <<-Gemfile.strip_heredoc
expect(content_of("gemfiles/ruby_version.gemfile")).to eq <<-GEMFILE.strip_heredoc
Copy link

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [86/80]


expect(content_of 'gemfiles/breakfast.gemfile').to eq <<-Gemfile.strip_heredoc
expect(content_of("gemfiles/breakfast.gemfile")).to eq <<-GEMFILE.strip_heredoc
Copy link

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [83/80]

select { |gem| gem.include?(gem_name) }
installed_gem = Dir.glob("tmp/stage/#{path}/#{Gem.ruby_engine}/*/gems/*")
.map { |path| path.split("/").last }
.select { |gem| gem.include?(gem_name) }
Copy link

Choose a reason for hiding this comment

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

Layout/DotPosition: Place the . on the previous line, together with the method call receiver.
Layout/MultilineMethodCallIndentation: Use 2 (not 19) spaces for indenting an expression in an assignment spanning multiple lines.

map { |path| path.split('/').last }.
select { |gem| gem.include?(gem_name) }
installed_gem = Dir.glob("tmp/stage/#{path}/#{Gem.ruby_engine}/*/gems/*")
.map { |path| path.split("/").last }
Copy link

Choose a reason for hiding this comment

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

Layout/DotPosition: Place the . on the previous line, together with the method call receiver.
Layout/MultilineMethodCallIndentation: Use 2 (not 19) spaces for indenting an expression in an assignment spanning multiple lines.

"bundle install --gemfile='#{file('gemfiles/1.0.0.gemfile')}' " \
"--path #{file('vendor/appraisal')} --retry 1",
)
expect(output).to include("bundle install --gemfile='#{file('gemfiles/1.0.0.gemfile')}' --path #{file('vendor/appraisal')} --retry 1")
Copy link

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [140/80]

"bundle install --gemfile='#{file('gemfiles/1.0.0.gemfile')}' " \
"--retry 1 --full-index true"
)
expect(output).to include("bundle install --gemfile='#{file('gemfiles/1.0.0.gemfile')}' --retry 1 --full-index true")
Copy link

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [123/80]


expect(output).to include("bundle install --gemfile='#{file('gemfiles/1.0.0.gemfile')}'")
expect(output).not_to include("bundle install --gemfile='#{file('gemfiles/1.0.0.gemfile')}' --jobs=0")
expect(output).not_to include("bundle install --gemfile='#{file('gemfiles/1.0.0.gemfile')}' --jobs=1")
Copy link

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [108/80]

write_file 'test.rb', 'puts "Running: #{$dummy_version}"'
write_file 'test with spaces.rb', 'puts "Running: #{$dummy_version}"'
run "appraisal install"
write_file "test.rb", 'puts "Running: #{$dummy_version}"'
Copy link

Choose a reason for hiding this comment

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

Lint/InterpolationCheck: Interpolation in single quoted string detected. Use double quoted strings if you need interpolation.

expect(content_of 'gemfiles/dummy.gemfile.lock').to include 'dummy (1.0.1)'
expect(content_of 'gemfiles/dummy.gemfile.lock').to include 'dummy2 (1.0.0)'
expect(content_of("gemfiles/dummy.gemfile.lock")).to include "dummy (1.0.1)"
expect(content_of("gemfiles/dummy.gemfile.lock")).to include "dummy2 (1.0.0)"
Copy link

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [83/80]


expect(content_of 'gemfiles/dummy.gemfile.lock').to include 'dummy (1.0.1)'
expect(content_of 'gemfiles/dummy.gemfile.lock').to include 'dummy2 (1.0.0)'
expect(content_of("gemfiles/dummy.gemfile.lock")).to include "dummy (1.0.1)"
Copy link

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [82/80]

expect(content_of 'gemfiles/dummy.gemfile.lock').to include 'dummy (1.0.1)'
expect(content_of 'gemfiles/dummy.gemfile.lock').to include 'dummy2 (1.0.1)'
expect(content_of("gemfiles/dummy.gemfile.lock")).to include "dummy (1.0.1)"
expect(content_of("gemfiles/dummy.gemfile.lock")).to include "dummy2 (1.0.1)"
Copy link

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [83/80]


expect(output).to include("gemfiles/dummy.gemfile bundle update")
expect(content_of 'gemfiles/dummy.gemfile.lock').to include 'dummy (1.0.1)'
expect(content_of 'gemfiles/dummy.gemfile.lock').to include 'dummy2 (1.0.1)'
expect(content_of("gemfiles/dummy.gemfile.lock")).to include "dummy (1.0.1)"
Copy link

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [82/80]

expect(content_of "gemfiles/1.2.0.gemfile").to include('gem "bacon", "1.2.0"')
expect(content_of("gemfiles/1.0.0.gemfile")).to include('gem "bacon", "1.0.0"')
expect(content_of("gemfiles/1.1.0.gemfile")).to include('gem "bacon", "1.1.0"')
expect(content_of("gemfiles/1.2.0.gemfile")).to include('gem "bacon", "1.2.0"')
Copy link

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [83/80]

expect(content_of "gemfiles/1.1.0.gemfile").to include('gem "bacon", "1.1.0"')
expect(content_of "gemfiles/1.2.0.gemfile").to include('gem "bacon", "1.2.0"')
expect(content_of("gemfiles/1.0.0.gemfile")).to include('gem "bacon", "1.0.0"')
expect(content_of("gemfiles/1.1.0.gemfile")).to include('gem "bacon", "1.1.0"')
Copy link

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [83/80]

expect(content_of "gemfiles/1.0.0.gemfile").to include('gem "bacon", "1.0.0"')
expect(content_of "gemfiles/1.1.0.gemfile").to include('gem "bacon", "1.1.0"')
expect(content_of "gemfiles/1.2.0.gemfile").to include('gem "bacon", "1.2.0"')
expect(content_of("gemfiles/1.0.0.gemfile")).to include('gem "bacon", "1.0.0"')
Copy link

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [83/80]


expect(content_of 'gemfiles/english.gemfile').to eq <<-Gemfile.strip_heredoc
expect(content_of("gemfiles/english.gemfile")).to eq <<-GEMFILE.strip_heredoc
Copy link

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [81/80]


expect(content_of 'gemfiles/japanese.gemfile').to eq <<-Gemfile.strip_heredoc
expect(content_of("gemfiles/japanese.gemfile")).to eq <<-GEMFILE.strip_heredoc
Copy link

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [82/80]

)).to eq("Gemfile: #{gemfile_relative_path}")
end

it "returns the heading with the relative lockfile path" do
expect(described_class.send(
:customize,
"Gemfile: %{relative_lockfile_path}", # rubocop:disable Style/FormatStringToken, Metrics/LineLength
appraisal,
"Gemfile: %{relative_lockfile_path}", # rubocop:disable Style/FormatStringToken
Copy link

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [94/80]

"Gemfile: %{relative_gemfile_path}", # rubocop:disable Style/FormatStringToken, Metrics/LineLength
appraisal,
"Gemfile: %{relative_gemfile_path}", # rubocop:disable Style/FormatStringToken
appraisal
Copy link

Choose a reason for hiding this comment

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

Style/TrailingCommaInArguments: Put a comma after the last parameter of a multiline method call.

)).to eq("Lockfile: #{lockfile_full_path}")
end

it "returns the heading with the relative gemfile path" do
expect(described_class.send(
:customize,
"Gemfile: %{relative_gemfile_path}", # rubocop:disable Style/FormatStringToken, Metrics/LineLength
appraisal,
"Gemfile: %{relative_gemfile_path}", # rubocop:disable Style/FormatStringToken
Copy link

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [93/80]

"Lockfile: %{lockfile_path}", # rubocop:disable Style/FormatStringToken, Metrics/LineLength
appraisal,
"Lockfile: %{lockfile_path}", # rubocop:disable Style/FormatStringToken
appraisal
Copy link

Choose a reason for hiding this comment

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

Style/TrailingCommaInArguments: Put a comma after the last parameter of a multiline method call.

)).to eq("Lockfile: #{lockfile}")
end

it "returns the heading with the lockfile path" do
expect(described_class.send(
:customize,
"Lockfile: %{lockfile_path}", # rubocop:disable Style/FormatStringToken, Metrics/LineLength
appraisal,
"Lockfile: %{lockfile_path}", # rubocop:disable Style/FormatStringToken
Copy link

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [86/80]

"Lockfile: %{lockfile}", # rubocop:disable Style/FormatStringToken, Metrics/LineLength
appraisal,
"Lockfile: %{lockfile}", # rubocop:disable Style/FormatStringToken
appraisal
Copy link

Choose a reason for hiding this comment

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

Style/TrailingCommaInArguments: Put a comma after the last parameter of a multiline method call.

)).to eq("Gemfile: #{gemfile_full_path}")
end

it "returns the heading with the lockfile name" do
expect(described_class.send(
:customize,
"Lockfile: %{lockfile}", # rubocop:disable Style/FormatStringToken, Metrics/LineLength
appraisal,
"Lockfile: %{lockfile}", # rubocop:disable Style/FormatStringToken
Copy link

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [81/80]

"Gemfile: %{gemfile_path}", # rubocop:disable Style/FormatStringToken, Metrics/LineLength
appraisal,
"Gemfile: %{gemfile_path}", # rubocop:disable Style/FormatStringToken
appraisal
Copy link

Choose a reason for hiding this comment

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

Style/TrailingCommaInArguments: Put a comma after the last parameter of a multiline method call.

)).to eq("Gemfile: #{gemfile}")
end

it "returns the heading with the gemfile path" do
expect(described_class.send(
:customize,
"Gemfile: %{gemfile_path}", # rubocop:disable Style/FormatStringToken, Metrics/LineLength
appraisal,
"Gemfile: %{gemfile_path}", # rubocop:disable Style/FormatStringToken
Copy link

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [84/80]

)).to eq("Appraisal: #{appraisal_name}")
end

it "returns the heading with the gemfile name" do
expect(described_class.send(
:customize,
"Gemfile: %{gemfile}", # rubocop:disable Style/FormatStringToken
appraisal,
appraisal
Copy link

Choose a reason for hiding this comment

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

Style/TrailingCommaInArguments: Put a comma after the last parameter of a multiline method call.

Appraisal::Utils.prefix_path("https://github.com/bacon/bacon.git")
).to eq("https://github.com/bacon/bacon.git")
expect(Appraisal::Utils.prefix_path("[email protected]:bacon/bacon.git")).to eq "[email protected]:bacon/bacon.git"
expect(Appraisal::Utils.prefix_path("git://github.com/bacon/bacon.git")).to eq "git://github.com/bacon/bacon.git"
Copy link

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [119/80]

expect(
Appraisal::Utils.prefix_path("https://github.com/bacon/bacon.git")
).to eq("https://github.com/bacon/bacon.git")
expect(Appraisal::Utils.prefix_path("[email protected]:bacon/bacon.git")).to eq "[email protected]:bacon/bacon.git"
Copy link

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [115/80]

expect(Appraisal::Utils.prefix_path("./tmp/./appraisal././")).
to eq "../tmp/appraisal./"
it "strips out './' from path" do
expect(Appraisal::Utils.prefix_path("./tmp/./appraisal././")).to eq "../tmp/appraisal./"
Copy link

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [94/80]

expect(Appraisal::Utils.format_arguments(arguments)).to eq(
':foo, bar: { baz: "ball" }'
)
expect(Appraisal::Utils.format_arguments(arguments)).to eq(':foo, bar: { baz: "ball" }')
Copy link

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [94/80]

expect(Appraisal::Utils.format_string(hash)).
to eq('"baz" => { ball: "boo" }')
hash = { "baz" => { ball: "boo" } }
expect(Appraisal::Utils.format_string(hash)).to eq('"baz" => { ball: "boo" }')
Copy link

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [84/80]

ENV["APPRAISAL_UNDER_TEST"] = "1"

RSpec.configure do |config|
config.raise_errors_for_deprecations!

config.define_derived_metadata(:file_path => %r{spec\/acceptance\/}) do |metadata|
config.define_derived_metadata(file_path: %r{spec\/acceptance\/}) do |metadata|
Copy link

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [81/80]

@@ -160,10 +167,10 @@ def run(command, raise_on_error = true)
end

if raise_on_error && exitstatus != 0
raise RuntimeError, <<-error_message.strip_heredoc
raise RuntimeError, <<-ERROR_MESSAGE.strip_heredoc
Copy link

Choose a reason for hiding this comment

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

Style/RedundantException: Redundant RuntimeError argument can be removed.

@n-rodriguez
Copy link
Contributor Author

@nickcharlton I've rebased on main branch, it's almost green 😄

@nickcharlton
Copy link
Member

Nice! This is a huge amount of work, thank you!

However, I'm going to be really annoying and request we break this out into a bunch of different PRs. I'm happy to do that if you'd rather not, though:

  1. Move appraisal script to 'exe' directory
  2. Add rspec and bundler binstub
  3. Disable RSpec monkey patching
  4. Skip broken tests
  5. Split bundler dir from gems build dir
  6. Improve debug output
  7. Silence Bundler warnings
  8. Remove useless file
  9. Ignore MacOS files
  10. Move development gems in Gemfile
  11. Cleanup gemspec
  12. Coding style [all of them together is fine enough]
  13. Fix: make .strip_heredoc private
  14. Add GithubAction config [we can then mark it to close all of the others, also assuming this needs to come last?]

@nickcharlton
Copy link
Member

I'm going to close this now, as we broke all of these out into different PRs.

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.

2 participants