From 79fa893d38c5468aa990a3395f187b2447a28ecd Mon Sep 17 00:00:00 2001 From: Pat Allan Date: Fri, 9 Feb 2024 12:29:29 +1100 Subject: [PATCH 1/4] =?UTF-8?q?Remove=20uses=20of=20ActiveSupport=E2=80=99?= =?UTF-8?q?s=20blank=3F/present=3F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/buildkite/test_collector.rb | 1 - lib/buildkite/test_collector/rspec_plugin/reporter.rb | 8 ++++++-- lib/buildkite/test_collector/rspec_plugin/trace.rb | 2 +- .../test_collector/test_links_plugin/formatter.rb | 2 +- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/lib/buildkite/test_collector.rb b/lib/buildkite/test_collector.rb index 7abd033..55cf666 100644 --- a/lib/buildkite/test_collector.rb +++ b/lib/buildkite/test_collector.rb @@ -12,7 +12,6 @@ module TestCollector require "timeout" require "tmpdir" -require "active_support/core_ext/object/blank" require "active_support/core_ext/hash/indifferent_access" require "active_support/notifications" diff --git a/lib/buildkite/test_collector/rspec_plugin/reporter.rb b/lib/buildkite/test_collector/rspec_plugin/reporter.rb index 5860bcf..609548b 100644 --- a/lib/buildkite/test_collector/rspec_plugin/reporter.rb +++ b/lib/buildkite/test_collector/rspec_plugin/reporter.rb @@ -40,6 +40,10 @@ def dump_summary(_notification) RSpec::Core::MultipleExceptionError ] + def blank?(string) + string.nil? || string.strip.empty? + end + def failure_info(notification) failure_expanded = [] @@ -73,9 +77,9 @@ def failure_info(notification) def format_message_lines(message_lines) message_lines.map! { |l| strip_diff_colors(l) } # the first line is sometimes blank, depending on the error reported - message_lines.shift if message_lines.first.blank? + message_lines.shift if blank?(message_lines.first) # the last line is sometimes blank, depending on the error reported - message_lines.pop if message_lines.last.blank? + message_lines.pop if blank?(message_lines.last) message_lines end diff --git a/lib/buildkite/test_collector/rspec_plugin/trace.rb b/lib/buildkite/test_collector/rspec_plugin/trace.rb index 3a9b755..d45decd 100644 --- a/lib/buildkite/test_collector/rspec_plugin/trace.rb +++ b/lib/buildkite/test_collector/rspec_plugin/trace.rb @@ -60,7 +60,7 @@ def file_name end def shared_example? - example.metadata[:shared_group_inclusion_backtrace].any? + !example.metadata[:shared_group_inclusion_backtrace].empty? end def shared_example_call_location diff --git a/lib/buildkite/test_collector/test_links_plugin/formatter.rb b/lib/buildkite/test_collector/test_links_plugin/formatter.rb index a3a4646..c6e9349 100644 --- a/lib/buildkite/test_collector/test_links_plugin/formatter.rb +++ b/lib/buildkite/test_collector/test_links_plugin/formatter.rb @@ -10,7 +10,7 @@ def initialize(output) def dump_failures(notification) # Do not display summary if no failed examples - return unless notification.failed_examples.present? + return if notification.failed_examples.empty? # Check if a Test Analytics token is set return unless Buildkite::TestCollector.api_token From d3246b9514e2a400e455329252f02c80a2c60a20 Mon Sep 17 00:00:00 2001 From: Pat Allan Date: Fri, 9 Feb 2024 12:30:21 +1100 Subject: [PATCH 2/4] Remove uses of with_indifferent_access The tests are fine without them. --- lib/buildkite/test_collector.rb | 1 - lib/buildkite/test_collector/minitest_plugin/trace.rb | 2 +- lib/buildkite/test_collector/rspec_plugin/trace.rb | 2 +- lib/buildkite/test_collector/tracer.rb | 2 +- 4 files changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/buildkite/test_collector.rb b/lib/buildkite/test_collector.rb index 55cf666..97e1df4 100644 --- a/lib/buildkite/test_collector.rb +++ b/lib/buildkite/test_collector.rb @@ -12,7 +12,6 @@ module TestCollector require "timeout" require "tmpdir" -require "active_support/core_ext/hash/indifferent_access" require "active_support/notifications" require_relative "test_collector/version" diff --git a/lib/buildkite/test_collector/minitest_plugin/trace.rb b/lib/buildkite/test_collector/minitest_plugin/trace.rb index c0e53e3..e474bd6 100644 --- a/lib/buildkite/test_collector/minitest_plugin/trace.rb +++ b/lib/buildkite/test_collector/minitest_plugin/trace.rb @@ -38,7 +38,7 @@ def as_hash failure_reason: failure_reason, failure_expanded: failure_expanded, history: history, - ).with_indifferent_access.select { |_, value| !value.nil? } + ).select { |_, value| !value.nil? } end private diff --git a/lib/buildkite/test_collector/rspec_plugin/trace.rb b/lib/buildkite/test_collector/rspec_plugin/trace.rb index d45decd..55415c5 100644 --- a/lib/buildkite/test_collector/rspec_plugin/trace.rb +++ b/lib/buildkite/test_collector/rspec_plugin/trace.rb @@ -32,7 +32,7 @@ def as_hash failure_reason: failure_reason, failure_expanded: failure_expanded, history: history, - ).with_indifferent_access.select { |_, value| !value.nil? } + ).select { |_, value| !value.nil? } end private diff --git a/lib/buildkite/test_collector/tracer.rb b/lib/buildkite/test_collector/tracer.rb index c41a14a..bc4d3ed 100644 --- a/lib/buildkite/test_collector/tracer.rb +++ b/lib/buildkite/test_collector/tracer.rb @@ -43,7 +43,7 @@ def as_hash duration: end_at - start_at, detail: detail, children: children.map(&:as_hash), - }.with_indifferent_access + } end end From f37d79af3b1239dc15619b6d6305f7648d9ecc60 Mon Sep 17 00:00:00 2001 From: Pat Allan Date: Fri, 9 Feb 2024 12:31:45 +1100 Subject: [PATCH 3/4] Tracing ActiveRecord if ActiveSupport is present MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the suite that’s being monitored doesn’t have ActiveSupport, then it definitely doesn’t have ActiveRecord, and so no tracing is required. --- lib/buildkite/test_collector.rb | 6 ++++-- spec/spec_helper.rb | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/buildkite/test_collector.rb b/lib/buildkite/test_collector.rb index 97e1df4..4f10b3a 100644 --- a/lib/buildkite/test_collector.rb +++ b/lib/buildkite/test_collector.rb @@ -12,8 +12,6 @@ module TestCollector require "timeout" require "tmpdir" -require "active_support/notifications" - require_relative "test_collector/version" require_relative "test_collector/error" require_relative "test_collector/ci" @@ -70,6 +68,10 @@ def self.enable_tracing! Buildkite::TestCollector::Network.configure Buildkite::TestCollector::Object.configure + return unless defined?(ActiveSupport) + + require "active_support/notifications" + ActiveSupport::Notifications.subscribe("sql.active_record") do |name, start, finish, id, payload| Buildkite::TestCollector::Uploader.tracer&.backfill(:sql, finish - start, **{ query: payload[:sql] }) end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index aae93ad..69fa230 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "buildkite/test_collector" +require "active_support/notifications" Dir["spec/support/**/*.rb"].each { |f| require File.expand_path(f) } From 5530fd159d53594deaa868a7c6223be82829c961 Mon Sep 17 00:00:00 2001 From: Pat Allan Date: Fri, 9 Feb 2024 12:32:13 +1100 Subject: [PATCH 4/4] Shift ActiveSupport to a development dependency And explicitly require OpenSSL (presuming ActiveSupport was doing that for us otherwise). --- Gemfile.lock | 2 +- buildkite-test_collector.gemspec | 3 +-- lib/buildkite/test_collector.rb | 1 + 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index a25b383..651b743 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -2,7 +2,6 @@ PATH remote: . specs: buildkite-test_collector (2.4.0) - activesupport (>= 4.2) GEM remote: https://rubygems.org/ @@ -38,6 +37,7 @@ PLATFORMS ruby DEPENDENCIES + activesupport (>= 4.2) buildkite-test_collector! rake (~> 13.0) rspec (~> 3.0) diff --git a/buildkite-test_collector.gemspec b/buildkite-test_collector.gemspec index c4d98d2..2c27b19 100644 --- a/buildkite-test_collector.gemspec +++ b/buildkite-test_collector.gemspec @@ -24,8 +24,7 @@ Gem::Specification.new do |spec| spec.required_ruby_version = Gem::Requirement.new(">= 2.3.0") - spec.add_dependency "activesupport", ">= 4.2" - + spec.add_development_dependency "activesupport", ">= 4.2" spec.add_development_dependency "rspec-core", '~> 3.10' spec.add_development_dependency "rspec-expectations", '~> 3.10' end diff --git a/lib/buildkite/test_collector.rb b/lib/buildkite/test_collector.rb index 4f10b3a..751bc86 100644 --- a/lib/buildkite/test_collector.rb +++ b/lib/buildkite/test_collector.rb @@ -8,6 +8,7 @@ module TestCollector require "json" require "logger" require "net/http" +require "openssl" require "time" require "timeout" require "tmpdir"