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

[CIVIS-7948] Test suite level visibility instrumentation for Minitest framework #92

Merged
merged 11 commits into from
Jan 3, 2024
Merged
Show file tree
Hide file tree
Changes from 10 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
2 changes: 2 additions & 0 deletions .standard_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,5 @@ ignore:
- Performance/UnfreezeString
- Appraisals:
- Style/Alias
- spec/datadog/ci/contrib/minitest/instrumentation_spec.rb:
- Lint/ConstantDefinitionInBlock
1 change: 1 addition & 0 deletions Steepfile
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,5 @@ target :lib do
library "rspec"
library "cucumber"
library "msgpack"
library "weakref"
end
1 change: 1 addition & 0 deletions lib/datadog/ci/configuration/components.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ def activate_ci!(settings)
if test_visibility_transport
writer_options[:transport] = test_visibility_transport
writer_options[:shutdown_timeout] = 60
writer_options[:buffer_size] = 10_000

settings.tracing.test_mode.async = true
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class Settings < Datadog::CI::Contrib::Settings

option :service_name do |o|
o.type :string
o.default { Datadog.configuration.service_without_fallback || Ext::SERVICE_NAME }
o.default { Datadog.configuration.service_without_fallback || Ext::DEFAULT_SERVICE_NAME }
end

# @deprecated Will be removed in 1.0
Expand Down
10 changes: 6 additions & 4 deletions lib/datadog/ci/contrib/minitest/ext.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,15 @@ module Minitest
# Minitest integration constants
# TODO: mark as `@public_api` when GA, to protect from resource and tag name changes.
module Ext
APP = "minitest"
ENV_ENABLED = "DD_TRACE_MINITEST_ENABLED"
ENV_OPERATION_NAME = "DD_TRACE_MINITEST_OPERATION_NAME"

FRAMEWORK = "minitest"

DEFAULT_SERVICE_NAME = "minitest"

# TODO: remove in 1.0
ENV_OPERATION_NAME = "DD_TRACE_MINITEST_OPERATION_NAME"
OPERATION_NAME = "minitest.test"
SERVICE_NAME = "minitest"
TEST_TYPE = "test"
end
end
end
Expand Down
64 changes: 48 additions & 16 deletions lib/datadog/ci/contrib/minitest/hooks.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

require_relative "../../ext/test"
require_relative "ext"
require_relative "suite"

module Datadog
module CI
Expand All @@ -11,47 +12,78 @@ module Minitest
module Hooks
def before_setup
super
return unless configuration[:enabled]
return unless datadog_configuration[:enabled]

test_name = "#{class_name}##{name}"

path, = method(name).source_location
test_suite = Pathname.new(path.to_s).relative_path_from(Pathname.pwd).to_s
test_suite_name = Suite.name(self.class, name)
if parallel?
test_suite_name = "#{test_suite_name} (#{name} concurrently)"

# for parallel execution we need to start a new test suite for each test
CI.start_test_suite(test_suite_name)
end

CI.start_test(
test_name,
test_suite,
test_suite_name,
tags: {
CI::Ext::Test::TAG_FRAMEWORK => Ext::FRAMEWORK,
CI::Ext::Test::TAG_FRAMEWORK_VERSION => CI::Contrib::Minitest::Integration.version.to_s,
CI::Ext::Test::TAG_TYPE => Ext::TEST_TYPE
CI::Ext::Test::TAG_TYPE => CI::Ext::Test::TEST_TYPE
},
service: configuration[:service_name]
service: datadog_configuration[:service_name]
)
end

def after_teardown
test_span = CI.active_test
return super unless test_span

finish_test(test_span, result_code)
if parallel?
finish_test_suite(test_span.test_suite, result_code)
end

super
end

private

def finish_test(test_span, result_code)
finish_with_result(test_span, result_code)

# mark test suite as failed if any test failed
if test_span.failed?
test_suite = test_span.test_suite
test_suite.failed! if test_suite
end
end

def finish_test_suite(test_suite, result_code)
return unless test_suite

finish_with_result(test_suite, result_code)
end

def finish_with_result(span, result_code)
case result_code
when "."
test_span.passed!
span.passed!
when "E", "F"
test_span.failed!(exception: failure)
span.failed!(exception: failure)
when "S"
test_span.skipped!(reason: failure.message)
span.skipped!(reason: failure.message)
end

test_span.finish

super
span.finish
end

private
def parallel?
self.class.test_order == :parallel
end

def configuration
::Datadog.configuration.ci[:minitest]
def datadog_configuration
Datadog.configuration.ci[:minitest]
end
end
end
Expand Down
7 changes: 7 additions & 0 deletions lib/datadog/ci/contrib/minitest/patcher.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# frozen_string_literal: true

require_relative "hooks"
require_relative "runnable"

module Datadog
module CI
Expand All @@ -17,7 +18,13 @@ def target_version
end

def patch
require_relative "plugin"

::Minitest::Test.include(Hooks)
::Minitest.include(Plugin)
::Minitest::Runnable.include(Runnable)

::Minitest.extensions << "datadog_ci"
end
end
end
Expand Down
73 changes: 73 additions & 0 deletions lib/datadog/ci/contrib/minitest/plugin.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
# frozen_string_literal: true

require "weakref"

require_relative "../../ext/test"
require_relative "ext"

module Datadog
module CI
module Contrib
module Minitest
module Plugin
def self.included(base)
base.extend(ClassMethods)
end

class DatadogReporter < ::Minitest::AbstractReporter
def initialize(minitest_reporter)
# This creates circular reference as minitest_reporter also holds reference to DatadogReporter.
# To make sure that minitest_reporter can be garbage collected, we use WeakRef.
@reporter = WeakRef.new(minitest_reporter)
end

def report
active_test_session = CI.active_test_session
active_test_module = CI.active_test_module

return unless @reporter.weakref_alive?
return if active_test_session.nil? || active_test_module.nil?

if @reporter.passed?
active_test_module.passed!
active_test_session.passed!
else
active_test_module.failed!
active_test_session.failed!
end

active_test_module.finish
active_test_session.finish

nil
end
end

module ClassMethods
def plugin_datadog_ci_init(*)
return unless datadog_configuration[:enabled]

test_session = CI.start_test_session(
tags: {
CI::Ext::Test::TAG_FRAMEWORK => Ext::FRAMEWORK,
CI::Ext::Test::TAG_FRAMEWORK_VERSION => CI::Contrib::Minitest::Integration.version.to_s,
CI::Ext::Test::TAG_TYPE => CI::Ext::Test::TEST_TYPE
},
service: datadog_configuration[:service_name]
)
CI.start_test_module(test_session.name)

reporter.reporters << DatadogReporter.new(reporter)
end

private

def datadog_configuration
Datadog.configuration.ci[:minitest]
end
end
end
end
end
end
end
46 changes: 46 additions & 0 deletions lib/datadog/ci/contrib/minitest/runnable.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
require_relative "suite"

module Datadog
module CI
module Contrib
module Minitest
module Runnable
def self.included(base)
base.singleton_class.prepend(ClassMethods)
end

module ClassMethods
def run(*)
return super unless datadog_configuration[:enabled]
return super if parallel?

method = runnable_methods.first
return super if method.nil?

test_suite_name = Suite.name(self, method)

test_suite = Datadog::CI.start_test_suite(test_suite_name)
test_suite.passed! # will be overridden if any test fails

Choose a reason for hiding this comment

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

Does it mean a test suite can never have 'skipped' status? Not sure what the intended behaviour is, mentioning it just in case

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I haven't implemented SKIP for test suites in any of the instrumentations right now. Is it heavily used by other libraries (if we don't include ITR)?

Choose a reason for hiding this comment

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

In Java the suite status is SKIP if it contains 0 test cases, or if every test case in it is skipped. Not sure about other languages

Copy link
Member Author

Choose a reason for hiding this comment

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

thank you! I will add a note for the future improvements


results = super

test_suite.finish

results
end

private

def parallel?
test_order == :parallel
end

def datadog_configuration
Datadog.configuration.ci[:minitest]
end
end
end
end
end
end
end
20 changes: 20 additions & 0 deletions lib/datadog/ci/contrib/minitest/suite.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# frozen_string_literal: true

module Datadog
module CI
module Contrib
module Minitest
# Minitest integration constants
# TODO: mark as `@public_api` when GA, to protect from resource and tag name changes.
module Suite
def self.name(klass, method_name)
source_location, = klass.instance_method(method_name).source_location
source_file_path = Pathname.new(source_location.to_s).relative_path_from(Pathname.pwd).to_s

Choose a reason for hiding this comment

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

Looks like it's implicitly assumed here that the current dir is always the same as the repo root. Not sure if the two can be different in Ruby world. If they can, things like source code integration and ITR will break in the backend

Copy link
Member Author

Choose a reason for hiding this comment

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

I haven't yet encountered cases where this wouldn't be true for Ruby as both bin/rails and rake command are always executed from the root. Note that this is actually a test suite name (which could be any arbitrary string), not the source code integration (which I will add together with ITR). For source code integration, I will check for the repository root.


"#{klass.name} at #{source_file_path}"
end
end
end
end
end
end
2 changes: 1 addition & 1 deletion lib/datadog/ci/contrib/rspec/integration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ def self.version
end

def self.loaded?
!defined?(::RSpec).nil? && !defined?(::RSpec::Core).nil? && \
!defined?(::RSpec).nil? && !defined?(::RSpec::Core).nil? &&
!defined?(::RSpec::Core::Example).nil?
end

Expand Down
2 changes: 1 addition & 1 deletion lib/datadog/ci/contrib/settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def [](name)
end

def []=(name, value)
respond_to?("#{name}=") ? send("#{name}=", value) : set_option(name, value)
respond_to?(:"#{name}=") ? send(:"#{name}=", value) : set_option(name, value)
end
end
end
Expand Down
24 changes: 24 additions & 0 deletions lib/datadog/ci/span.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,30 @@ def span_type
tracer_span.type
end

# Checks whether span status is not set yet.
# @return [bool] true if span does not have status
def undefined?
tracer_span.get_tag(Ext::Test::TAG_STATUS).nil?
end

# Checks whether span status is PASS.
# @return [bool] true if span status is PASS
def passed?
tracer_span.get_tag(Ext::Test::TAG_STATUS) == Ext::Test::Status::PASS
end

# Checks whether span status is FAIL.
# @return [bool] true if span status is FAIL
def failed?
tracer_span.get_tag(Ext::Test::TAG_STATUS) == Ext::Test::Status::FAIL
end

# Checks whether span status is SKIP.
# @return [bool] true if span status is SKIP
def skipped?
tracer_span.get_tag(Ext::Test::TAG_STATUS) == Ext::Test::Status::SKIP
end

# Sets the status of the span to "pass".
# @return [void]
def passed!
Expand Down
8 changes: 8 additions & 0 deletions lib/datadog/ci/test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,14 @@ def finish
CI.deactivate_test(self)
end

# Running test suite that this test is part of (if any).
# @return [Datadog::CI::TestSuite] the test suite this test belongs to
# @return [nil] if the test suite is not found
def test_suite
suite_name = test_suite_name
CI.active_test_suite(suite_name) if suite_name
end

# Span id of the running test suite this test belongs to.
# @return [String] the span id of the test suite.
def test_suite_id
Expand Down
6 changes: 1 addition & 5 deletions sig/datadog/ci/contrib/minitest/ext.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ module Datadog
module Contrib
module Minitest
module Ext
APP: String

ENV_ENABLED: String

ENV_OPERATION_NAME: String
Expand All @@ -13,9 +11,7 @@ module Datadog

OPERATION_NAME: String

SERVICE_NAME: String

TEST_TYPE: String
DEFAULT_SERVICE_NAME: String
end
end
end
Expand Down
Loading