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

[WIP] brew search freezes on some GitHub API responses #2466

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 2 additions & 17 deletions Library/Homebrew/cask/lib/hbc/system_command.rb
Original file line number Diff line number Diff line change
Expand Up @@ -81,31 +81,16 @@ def each_output_line(&b)

write_input_to(raw_stdin)
raw_stdin.close_write
each_line_from [raw_stdout, raw_stderr], &b

::Utils::IOSelector
.each_line_from(stdout: raw_stdout, stderr: raw_stderr, &b)
@processed_status = raw_wait_thr.value
end

def write_input_to(raw_stdin)
[*options[:input]].each { |line| raw_stdin.print line }
end

def each_line_from(sources)
loop do
readable_sources = IO.select(sources)[0]
readable_sources.delete_if(&:eof?).first(1).each do |source|
type = ((source == sources[0]) ? :stdout : :stderr)
begin
yield(type, source.readline_nonblock || "")
rescue IO::WaitReadable, EOFError
next
end
end
break if readable_sources.empty?
end
sources.each(&:close_read)
end

def result
Result.new(command,
processed_output[:stdout],
Expand Down
3 changes: 3 additions & 0 deletions Library/Homebrew/test/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
require "rspec/its"
require "rspec/wait"
require "set"
require "English"

if ENV["HOMEBREW_TESTS_COVERAGE"]
require "simplecov"
Expand All @@ -20,6 +21,7 @@
require "tap"

require "test/support/helper/shutup"
require "test/support/helper/fake_curl"
require "test/support/helper/fixtures"
require "test/support/helper/formula"
require "test/support/helper/mktmpdir"
Expand All @@ -42,6 +44,7 @@
config.order = :random

config.include(Test::Helper::Shutup)
config.include(Test::Helper::FakeCurl)
config.include(Test::Helper::Fixtures)
config.include(Test::Helper::Formula)
config.include(Test::Helper::MkTmpDir)
Expand Down
Binary file not shown.
25 changes: 25 additions & 0 deletions Library/Homebrew/test/support/helper/fake_curl.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
require_relative "./mktmpdir"

module Test
module Helper
module FakeCurl
def with_fake_curl(shell_command)
saved_value = ENV["HOMEBREW_CURL"]

begin
mktmpdir do |path|
fake_curl_path = path/"fake_curl"
File.open(fake_curl_path, "w") do |file|
file.write("#! #{`which bash`}\n#{shell_command}\n")
end
FileUtils.chmod 0755, fake_curl_path
ENV["HOMEBREW_CURL"] = fake_curl_path
end
return yield
ensure
ENV["HOMEBREW_CURL"] = saved_value
end
end
end
end
end
137 changes: 137 additions & 0 deletions Library/Homebrew/test/utils/curl_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
require "utils/curl"

describe "in `utils/curl`" do
before(:example) { ENV["HOMEBREW_CURL"] = "" }

describe "the `curl_args` method" do
let(:saved_travis_env) { ENV["TRAVIS"] }
before(:example) { ENV["TRAVIS"] = "1" }

describe "given no options" do
subject { curl_args }

it { is_expected.to be_an(Array) }

its(:size) { is_expected.to eq(8) }

it { is_expected.to start_with("/usr/bin/curl") }

it {
is_expected.to contain_exactly(
"/usr/bin/curl",
"--remote-time",
"--location",
"--user-agent",
HOMEBREW_USER_AGENT_CURL,
"--progress-bar",
"--fail",
"--silent",
)
}
end

describe "with the `user_agent` option set to `:browser`" do
subject { curl_args(user_agent: :browser) }

it { is_expected.to be_an(Array) }

its(:size) { is_expected.to eq(8) }

it { is_expected.to start_with("/usr/bin/curl") }

it {
is_expected.to contain_exactly(
"/usr/bin/curl",
"--remote-time",
"--location",
"--user-agent",
HOMEBREW_USER_AGENT_FAKE_SAFARI,
"--progress-bar",
"--fail",
"--silent",
)
}
end

describe "with the `show_output` option enabled" do
subject { curl_args(show_output: true) }

it { is_expected.to be_an(Array) }

its(:size) { is_expected.to eq(5) }

it { is_expected.to start_with("/usr/bin/curl") }

it {
is_expected.to contain_exactly(
"/usr/bin/curl",
"--remote-time",
"--location",
"--user-agent",
HOMEBREW_USER_AGENT_CURL,
)
}
end

after(:example) { ENV["TRAVIS"] = saved_travis_env }
end

describe "the `curl_output` method" do
describe "when curl outputs something on STDOUT only" do
subject { with_fake_curl("seq 1 5") { curl_output } }

it { is_expected.to be_an(Array) }
its(:size) { is_expected.to eq(3) }

its([0]) { is_expected.to eq([1, 2, 3, 4, 5, nil].join("\n")) }
its([1]) { is_expected.to be_empty }
its([2]) { is_expected.to be_a_success }
end

describe "when curl outputs something on STDERR only" do
subject { with_fake_curl("seq 1 5 >&2") { curl_output } }

it { is_expected.to be_an(Array) }
its(:size) { is_expected.to eq(3) }

its([0]) { is_expected.to be_empty }
its([1]) { is_expected.to eq([1, 2, 3, 4, 5, nil].join("\n")) }
its([2]) { is_expected.to be_a_success }
end

describe "when curl outputs something on STDOUT and STDERR" do
subject {
with_fake_curl(<<-EOF.undent) { curl_output }
for i in $(seq 1 2 5); do
echo $i; echo $(($i + 1)) >&2
done
EOF
}

it { is_expected.to be_an(Array) }
its(:size) { is_expected.to eq(3) }

its([0]) { is_expected.to eq([1, 3, 5, nil].join("\n")) }
its([1]) { is_expected.to eq([2, 4, 6, nil].join("\n")) }
its([2]) { is_expected.to be_a_success }
end

describe "with a very long STDERR output" do
let(:shell_command) {
<<-EOF.undent
for i in $(seq 1 2 100000); do
echo $i; echo $(($i + 1)) >&2
done
EOF
}

it "returns without deadlocking" do
wait(15).for {
with_fake_curl(shell_command) { curl_output }
}.to end_with(an_object_satisfying(&:success?))
end
end
end

after(:example) { ENV["HOMEBREW_CURL"] = "" }
end
Loading