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.

Sign up for GitHub

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

Jump to bottom

formulary_spec: update API tests to avoid mocking #16697

Merged
merged 1 commit into from
Feb 19, 2024
Merged

formulary_spec: update API tests to avoid mocking #16697

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion Library/Homebrew/tap.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1169,7 +1169,8 @@ def formula_files_by_name
name, formula_hash = item
# If there's more than one item with the same path: use the longer one to prioritise more specific results.
existing_path = hash[name]
new_path = File.join(tap_path, formula_hash["ruby_source_path"]) # Pathname equivalent is slow in a tight loop
# Pathname equivalent is slow in a tight loop
new_path = File.join(tap_path, formula_hash.fetch("ruby_source_path"))
Comment on lines -1172 to +1173
Copy link
Contributor Author

Choose a reason for hiding this comment

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

File#join expects one or more strings while Array#join handles nil arguments gracefully.

irb(main):001:0> ["one", nil].join
=> "one"
irb(main):002:0> File.join("one", nil)
(irb):2:in `join': no implicit conversion of nil into String (TypeError)
        from (irb):2:in `<main>'                              
        from /Users/kevinrobell/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/irb-1.6.2/exe/irb:11:in `<top (required)>'
        from /Users/kevinrobell/.asdf/installs/ruby/3.2.2/bin/irb:25:in `load'
        from /Users/kevinrobell/.asdf/installs/ruby/3.2.2/bin/irb:25:in `<main>'

Copy link
Member

Choose a reason for hiding this comment

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

As long as the API JSON is current, "ruby_source_path" will always be populated, right?

Copy link
Member

Choose a reason for hiding this comment

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

In any case this doesn't change the behaviour in case of nil, only the error message.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As long as the API JSON is current, "ruby_source_path" will always be populated, right?

Correct.

In any case this doesn't change the behaviour in case of nil, only the error message.

Yep, and it also better shows intent.

hash[name] = Pathname(new_path) if existing_path.nil? || existing_path.to_s.length < new_path.length
end
end
Expand Down
5 changes: 2 additions & 3 deletions Library/Homebrew/test/formulary_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,6 @@ class Wrong#{described_class.class_s(formula_name)} < Formula

context "with installed Formula" do
before do
allow(described_class).to receive(:loader_for).and_call_original

# don't try to load/fetch gcc/glibc
allow(DevelopmentTools).to receive_messages(needs_libc_formula?: false, needs_compiler_formula?: false)
end
Expand Down Expand Up @@ -329,6 +327,7 @@ def formula_json_contents(extra_items = {})
"run_type" => "immediate",
"working_dir" => "/$HOME",
},
"ruby_source_path" => "Formula/#{formula_name}.rb",
}.merge(extra_items),
}
end
Expand Down Expand Up @@ -378,7 +377,7 @@ def formula_json_contents(extra_items = {})
end

before do
allow(described_class).to receive(:loader_for).and_return(described_class::FormulaAPILoader.new(formula_name))
ENV.delete("HOMEBREW_NO_INSTALL_FROM_API")

# don't try to load/fetch gcc/glibc
allow(DevelopmentTools).to receive_messages(needs_libc_formula?: false, needs_compiler_formula?: false)
Expand Down