Skip to content

Commit

Permalink
additionally use test.parameters when determining if test is skippable
Browse files Browse the repository at this point in the history
  • Loading branch information
anmarchenko committed Apr 19, 2024
1 parent 9990d61 commit c0ac3c9
Show file tree
Hide file tree
Showing 14 changed files with 46 additions and 23 deletions.
2 changes: 1 addition & 1 deletion lib/datadog/ci/contrib/minitest/hooks.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def before_setup
},
service: datadog_configuration[:service_name]
)
skip("skipped by ITR") if test_span&.skipped_by_itr?
skip(CI::Ext::Test::ITR_TEST_SKIP_REASON) if test_span&.skipped_by_itr?
end

def after_teardown
Expand Down
4 changes: 2 additions & 2 deletions lib/datadog/ci/contrib/rspec/example.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ def run(*)
},
service: datadog_configuration[:service_name]
) do |test_span|
result = super

test_span&.set_parameters({}, {"scoped_id" => metadata[:scoped_id]})

result = super

case execution_result.status
when :passed
test_span&.passed!
Expand Down
1 change: 1 addition & 0 deletions lib/datadog/ci/ext/test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ module Test
# could be either "test" or "suite" depending on whether we skip individual tests or whole suites
# we use test skipping for Ruby
ITR_TEST_SKIPPING_MODE = "test"
ITR_TEST_SKIP_REASON = "Skipped by Datadog's intelligent test runner"

# test status as recognized by Datadog
module Status
Expand Down
9 changes: 4 additions & 5 deletions lib/datadog/ci/itr/runner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -125,14 +125,13 @@ def stop_coverage(test)
def mark_if_skippable(test)
return if !enabled? || !skipping_tests?

test_full_name = Utils::TestRun.test_full_name(test.name, test.test_suite_name)

if @skippable_tests.include?(test_full_name)
skippable_test_id = Utils::TestRun.test_full_name(test.name, test.test_suite_name, test.parameters)
if @skippable_tests.include?(skippable_test_id)
test.set_tag(Ext::Test::TAG_ITR_SKIPPED_BY_ITR, "true")

Datadog.logger.debug { "Marked test as skippable: #{test_full_name}" }
Datadog.logger.debug { "Marked test as skippable: #{skippable_test_id}" }
else
Datadog.logger.debug { "Test is not skippable: #{test_full_name}" }
Datadog.logger.debug { "Test is not skippable: #{skippable_test_id}" }
end
end

Expand Down
2 changes: 1 addition & 1 deletion lib/datadog/ci/itr/skippable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def tests
next unless test_data["type"] == Ext::Test::ITR_TEST_SKIPPING_MODE

attrs = test_data["attributes"] || {}
res << Utils::TestRun.test_full_name(attrs["name"], attrs["suite"])
res << Utils::TestRun.test_full_name(attrs["name"], attrs["suite"], attrs["parameters"])
end

res
Expand Down
10 changes: 9 additions & 1 deletion lib/datadog/ci/test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ def skipped!(exception: nil, reason: nil)
record_test_result(Ext::Test::Status::SKIP)
end

# Sets the parameters for this test (e.g. Cucumber example or RSpec shared specs).
# Sets the parameters for this test (e.g. Cucumber example or RSpec specs).
# Parameters are needed to compute test fingerprint to distinguish between different tests having same names.
#
# @param [Hash] arguments the arguments that test accepts as key-value hash
Expand All @@ -115,6 +115,14 @@ def set_parameters(arguments, metadata = {})
)
end

# Gets the parameters for this test (e.g. Cucumber example or RSpec specs) as a serialized JSON.
#
# @return [String] the serialized JSON of the parameters
# @return [nil] if this test does not have parameters
def parameters
get_tag(Ext::Test::TAG_PARAMETERS)
end

private

def record_test_result(datadog_status)
Expand Down
4 changes: 2 additions & 2 deletions lib/datadog/ci/utils/test_run.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ def self.command
@command = "#{$0} #{ARGV.join(" ")}"
end

def self.test_full_name(test_name, suite)
"#{suite}.#{test_name}"
def self.test_full_name(test_name, suite, parameters = nil)
"#{suite}.#{test_name}.#{parameters}"
end
end
end
Expand Down
2 changes: 2 additions & 0 deletions sig/datadog/ci/ext/test.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ module Datadog

ITR_TEST_SKIPPING_MODE: "test"

ITR_TEST_SKIP_REASON: String

module Status
PASS: "pass"

Expand Down
1 change: 1 addition & 0 deletions sig/datadog/ci/test.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ module Datadog
def test_session_id: () -> String?
def skipped_by_itr?: () -> bool
def source_file: () -> String?
def parameters: () -> String?

private

Expand Down
2 changes: 1 addition & 1 deletion sig/datadog/ci/utils/test_run.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module Datadog

def self.command: () -> String

def self.test_full_name: (String test_name, String? test_suite) -> String
def self.test_full_name: (String test_name, String? test_suite, ?String? parameters) -> String
end
end
end
Expand Down
12 changes: 6 additions & 6 deletions spec/datadog/ci/contrib/minitest/instrumentation_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,7 @@ def test_pass_other
context "when ITR skips tests" do
context "single skipped test" do
let(:itr_skippable_tests) do
Set.new(["SomeTest at spec/datadog/ci/contrib/minitest/instrumentation_spec.rb.test_pass"])
Set.new(["SomeTest at spec/datadog/ci/contrib/minitest/instrumentation_spec.rb.test_pass."])
end

it "skips a single test" do
Expand All @@ -522,8 +522,8 @@ def test_pass_other
let(:itr_skippable_tests) do
Set.new(
[
"SomeTest at spec/datadog/ci/contrib/minitest/instrumentation_spec.rb.test_pass",
"SomeTest at spec/datadog/ci/contrib/minitest/instrumentation_spec.rb.test_pass_other"
"SomeTest at spec/datadog/ci/contrib/minitest/instrumentation_spec.rb.test_pass.",
"SomeTest at spec/datadog/ci/contrib/minitest/instrumentation_spec.rb.test_pass_other."
]
)
end
Expand Down Expand Up @@ -723,9 +723,9 @@ def test_b_2
let(:itr_skippable_tests) do
Set.new(
[
"TestA at spec/datadog/ci/contrib/minitest/instrumentation_spec.rb (test_a_1 concurrently).test_a_1",
"TestA at spec/datadog/ci/contrib/minitest/instrumentation_spec.rb (test_a_2 concurrently).test_a_2",
"TestB at spec/datadog/ci/contrib/minitest/instrumentation_spec.rb (test_b_2 concurrently).test_b_2"
"TestA at spec/datadog/ci/contrib/minitest/instrumentation_spec.rb (test_a_1 concurrently).test_a_1.",
"TestA at spec/datadog/ci/contrib/minitest/instrumentation_spec.rb (test_a_2 concurrently).test_a_2.",
"TestB at spec/datadog/ci/contrib/minitest/instrumentation_spec.rb (test_b_2 concurrently).test_b_2."
]
)
end
Expand Down
6 changes: 3 additions & 3 deletions spec/datadog/ci/itr/runner_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
fetch_skippable_tests: instance_double(
Datadog::CI::ITR::Skippable::Response,
correlation_id: "42",
tests: Set.new(["suite.test"])
tests: Set.new(["suite.test."])
)
)
end
Expand All @@ -77,7 +77,7 @@
expect(runner.skipping_tests?).to be true

expect(runner.correlation_id).to eq("42")
expect(runner.skippable_tests).to eq(Set.new(["suite.test"]))
expect(runner.skippable_tests).to eq(Set.new(["suite.test."]))

expect(git_worker).to have_received(:wait_until_done)
end
Expand Down Expand Up @@ -233,7 +233,7 @@
fetch_skippable_tests: instance_double(
Datadog::CI::ITR::Skippable::Response,
correlation_id: "42",
tests: Set.new(["suite.test", "suite2.test", "suite.test3"])
tests: Set.new(["suite.test.", "suite2.test.", "suite.test3."])
)
)
end
Expand Down
2 changes: 1 addition & 1 deletion spec/datadog/ci/itr/skippable_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@
it "parses the response" do
expect(response.ok?).to be true
expect(response.correlation_id).to eq("correlation_id_123")
expect(response.tests).to eq(Set.new(["test_suite_name.test_name"]))
expect(response.tests).to eq(Set.new(["test_suite_name.test_name.string"]))
end
end

Expand Down
12 changes: 12 additions & 0 deletions spec/datadog/ci/test_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -209,4 +209,16 @@
end
end
end

describe "#parameters" do
let(:parameters) { JSON.generate({arguments: {"foo" => "bar", "baz" => "qux"}, metadata: {}}) }

before do
allow(tracer_span).to receive(:get_tag).with("test.parameters").and_return(parameters)
end

it "returns the parameters" do
expect(ci_test.parameters).to eq(parameters)
end
end
end

0 comments on commit c0ac3c9

Please sign in to comment.