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

Action Cable integration test should soft fail on exit 3 #119

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
11 changes: 6 additions & 5 deletions lib/buildkite/config/rake_command.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,16 +106,17 @@ def rake(dir, task: "test", label: nil, service: "default", pre_steps: nil, env:

artifact_paths build_context.artifact_paths

if retry_on
automatic_retry_on(**retry_on)
else
automatic_retry_on(**build_context.automatic_retry_on)
[retry_on].flatten.compact.each do |retry_args|
automatic_retry_on(**retry_args)
Comment on lines +109 to +110
Copy link
Member Author

Choose a reason for hiding this comment

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

This is sugar to support passing a hash or array of hashes, as supported by the automatic retry attributes.

end
automatic_retry_on(**build_context.automatic_retry_on)
Copy link
Member Author

Choose a reason for hiding this comment

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

Before we would either set the given policy OR the default, now we always set both. There is some duplication in the rails-ci pipeline where we can remove the old retry policy commands now.


timeout_in_minutes build_context.timeout_in_minutes

if soft_fail || build_context.ruby.soft_fail?
if soft_fail.is_a?(TrueClass) || build_context.ruby.soft_fail?
soft_fail true
else
soft_fail([soft_fail].flatten.compact) if soft_fail
Comment on lines +116 to +119
Copy link
Member Author

Choose a reason for hiding this comment

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

Since soft_fail can be either a boolean, or an array of hashes. I still want the sugar to support both arrays or hashes, AND we have to consider if the Ruby version is soft failed.

I haven't tested what the result is if you for example, pass an empty array, and wanted to limit the amount of change here -- but there is probably a nicer way to write this.

end

if parallelism
Expand Down
2 changes: 1 addition & 1 deletion pipelines/rails-ci/pipeline.rb
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@
end

# ActionCable and ActiveJob integration tests
rake "actioncable", task: "test:integration", retry_on: { exit_status: -1, limit: 3 }
rake "actioncable", task: "test:integration", retry_on: { exit_status: 3, limit: 1 }, soft_fail: { exit_status: 3 }

if ruby == build_context.default_ruby
if build_context.rails_root.join("actionview/Rakefile").read.include?("task :ujs")
Expand Down
64 changes: 63 additions & 1 deletion test/buildkite_config/test_rake_command.rb
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,41 @@ def test_automatic_retry_on
end

assert_includes pipeline.to_h["steps"][0], "retry"
assert_equal({ "automatic" => [{ "limit" => 1, "exit_status" => 127 }] }, pipeline.to_h["steps"][0]["retry"])
assert_equal({ "automatic" => [{ "limit" => 1, "exit_status" => 127 }, { "limit" => 2, "exit_status" => -1 }] }, pipeline.to_h["steps"][0]["retry"])
end

def test_automatic_retry_on_default
pipeline = PipelineFixture.new do
build_context.ruby = Buildkite::Config::RubyConfig.new(version: Gem::Version.new("3.2"))
use Buildkite::Config::RakeCommand

build_context.stub(:rails_version, Gem::Version.new("7.1")) do
rake "test_automatic_retry_on"
end
end

assert_includes pipeline.to_h["steps"][0], "retry"
assert_equal({ "automatic" => [{ "limit" => 2, "exit_status" => -1 } ] }, pipeline.to_h["steps"][0]["retry"])
end

def test_automatic_retry_on_array
pipeline = PipelineFixture.new do
build_context.ruby = Buildkite::Config::RubyConfig.new(version: Gem::Version.new("3.2"))
use Buildkite::Config::RakeCommand

build_context.stub(:rails_version, Gem::Version.new("7.1")) do
rake "test_automatic_retry_on", retry_on: [{ limit: 1, exit_status: 127 }, { limit: 2, exit_status: 2 }, { limit: 3, exit_status: 3 }]
end
end

assert_includes pipeline.to_h["steps"][0], "retry"
assert_equal(
{ "automatic" =>
[{ "limit" => 1, "exit_status" => 127 },
{ "limit" => 2, "exit_status" => 2 },
{ "limit" => 3, "exit_status" => 3 },
{ "limit" => 2, "exit_status" => -1 }]
}, pipeline.to_h["steps"][0]["retry"])
end

def test_soft_fail
Expand All @@ -345,6 +379,34 @@ def test_soft_fail
assert_equal true, pipeline.to_h["steps"][0]["soft_fail"]
end

def test_soft_fail_hash
pipeline = PipelineFixture.new do
build_context.ruby = Buildkite::Config::RubyConfig.new(version: Gem::Version.new("3.2"))
use Buildkite::Config::RakeCommand

build_context.stub(:rails_version, Gem::Version.new("7.1")) do
rake "test_soft_fail", soft_fail: { exit_status: 127 }
end
end

assert_includes pipeline.to_h["steps"][0], "soft_fail"
assert_equal [{ "exit_status" => 127 }], pipeline.to_h["steps"][0]["soft_fail"]
end

def test_soft_fail_array
pipeline = PipelineFixture.new do
build_context.ruby = Buildkite::Config::RubyConfig.new(version: Gem::Version.new("3.2"))
use Buildkite::Config::RakeCommand

build_context.stub(:rails_version, Gem::Version.new("7.1")) do
rake "test_soft_fail", soft_fail: [{ exit_status: 127 }, { exit_status: 2 }]
end
end

assert_includes pipeline.to_h["steps"][0], "soft_fail"
assert_equal [{ "exit_status" => 127 }, { "exit_status" => 2 }], pipeline.to_h["steps"][0]["soft_fail"]
end

def test_soft_fail_ruby
pipeline = PipelineFixture.new do
build_context.ruby = Buildkite::Config::RubyConfig.new(version: Gem::Version.new("3.3"), soft_fail: true)
Expand Down