From 57d64741e4a690713382ae4525f579595b5f7576 Mon Sep 17 00:00:00 2001 From: Daniel Pepper Date: Sat, 28 Mar 2026 06:44:12 +0700 Subject: [PATCH 1/5] Global spec temp directory setup Use Dir.mktmpdir block form in spec_helper to automatically create and cleanup temp directories for all specs. Removes duplicate setup from individual spec files. Co-Authored-By: Claude Opus 4.5 --- spec/singed_spec.rb | 16 ++++------------ spec/spec_helper.rb | 9 +++++++++ spec/support/sidekiq.rb | 9 --------- 3 files changed, 13 insertions(+), 21 deletions(-) diff --git a/spec/singed_spec.rb b/spec/singed_spec.rb index 700bb6b..37db1bc 100644 --- a/spec/singed_spec.rb +++ b/spec/singed_spec.rb @@ -1,19 +1,12 @@ # frozen_string_literal: true -require "tempfile" - RSpec.describe Singed do around do |example| - original_output_directory = Singed.output_directory - Singed.output_directory = Dir.mktmpdir("singed-spec") original_enabled = Singed.enabled? - begin - example.run - ensure - Singed.output_directory = original_output_directory - Singed.enabled = original_enabled - Singed.instance_variable_set(:@current_flamegraph, nil) - end + example.run + ensure + Singed.enabled = original_enabled + Singed.instance_variable_set(:@current_flamegraph, nil) end describe ".start" do @@ -42,7 +35,6 @@ describe ".stop" do before do Singed.enabled = true - Singed.output_directory = Dir.mktmpdir("singed-spec") end it "returns nil when not profiling" do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 1850212..93d807f 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -15,8 +15,17 @@ # See https://rubydoc.info/gems/rspec-core/RSpec/Core/Configuration require "singed" +require "tmpdir" RSpec.configure do |config| + config.around do |example| + Dir.mktmpdir("singed-spec") do |dir| + Singed.output_directory = dir + example.run + end + ensure + Singed.output_directory = nil + end # rspec-expectations config goes here. You can use an alternate # assertion/expectation library such as wrong or the stdlib/minitest # assertions if you prefer. diff --git a/spec/support/sidekiq.rb b/spec/support/sidekiq.rb index 4634569..c813758 100644 --- a/spec/support/sidekiq.rb +++ b/spec/support/sidekiq.rb @@ -1,5 +1,4 @@ require "singed/sidekiq" -require "tempfile" RSpec.configure do |config| config.before(:suite) do @@ -16,14 +15,6 @@ ActiveJob::Base.queue_adapter = :sidekiq ActiveJob::Base.logger = Logger.new(nil) end - - config.around(:each, sidekiq: true) do |example| - orig_dir = Singed.output_directory - Singed.output_directory = Dir.mktmpdir("singed-sidekiq-spec") - example.run - ensure - Singed.output_directory = orig_dir - end end # Sidekiq doesn't invoke middlewares in inline testingmode, so we need to invoke it oursleves From 64cadf0fca9ce0d34fd027bc2eb8279cd13d7db5 Mon Sep 17 00:00:00 2001 From: Daniel Pepper Date: Tue, 19 Nov 2024 06:33:56 +0800 Subject: [PATCH 2/5] rack cleanup --- Gemfile | 1 + Gemfile.lock | 3 +++ lib/singed/rack_middleware.rb | 8 ++----- spec/singed/middleware_spec.rb | 41 ++++++++++++++++++++++++++++++++++ 4 files changed, 47 insertions(+), 6 deletions(-) create mode 100644 spec/singed/middleware_spec.rb diff --git a/Gemfile b/Gemfile index 97879fc..b83ed2e 100644 --- a/Gemfile +++ b/Gemfile @@ -6,6 +6,7 @@ source "https://rubygems.org" gemspec gem "activejob" +gem "rack-test" gem "rake", "~> 13.0" gem "rspec" gem "rubyzip" diff --git a/Gemfile.lock b/Gemfile.lock index 23aaef0..04ac319 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -48,6 +48,8 @@ GEM prism (1.9.0) racc (1.8.1) rack (3.2.5) + rack-test (2.2.0) + rack (>= 1.3) rainbow (3.1.1) rake (13.0.6) redis-client (0.26.4) @@ -118,6 +120,7 @@ PLATFORMS DEPENDENCIES activejob + rack-test rake (~> 13.0) rspec rubyzip diff --git a/lib/singed/rack_middleware.rb b/lib/singed/rack_middleware.rb index bce1e2f..f267974 100644 --- a/lib/singed/rack_middleware.rb +++ b/lib/singed/rack_middleware.rb @@ -9,15 +9,11 @@ def initialize(app) end def call(env) - status, headers, body = if capture_flamegraph?(env) - flamegraph do - @app.call(env) - end + if capture_flamegraph?(env) + flamegraph { @app.call(env) } else @app.call(env) end - - [status, headers, body] end def capture_flamegraph?(env) diff --git a/spec/singed/middleware_spec.rb b/spec/singed/middleware_spec.rb new file mode 100644 index 0000000..93abbcc --- /dev/null +++ b/spec/singed/middleware_spec.rb @@ -0,0 +1,41 @@ +describe Singed::RackMiddleware do + subject do + instance.call(env) + end + + let(:app) { ->(*) { [200, {'Content-Type' => 'text/plain'}, ['OK']] } } + let(:instance) { described_class.new(app) } + let(:env) { Rack::MockRequest.env_for('/', headers) } + let(:headers) { {} } + + it 'returns a proper rack response' do + status, _ = subject + expect(status).to eq 200 + end + + it 'does not capture a flamegraph by default' do + expect(instance).not_to receive(:flamegraph) + subject + end + + context 'when enabled' do + before { allow(instance).to receive(:capture_flamegraph?).and_return(true) } + + it 'captures a flamegraph' do + expect(instance).to receive(:flamegraph) + subject + end + end + + describe '.capture_flamegraph?' do + subject { instance.capture_flamegraph?(env) } + + it { is_expected.to be false } + + context 'when HTTP_X_SINGED is true' do + let(:headers) { { 'HTTP_X_SINGED' => 'true' } } + + it { is_expected.to be true } + end + end +end From 9c98aa4d53d3498a60636b89221e6082fdf68d4e Mon Sep 17 00:00:00 2001 From: Daniel Pepper Date: Mon, 9 Mar 2026 06:11:06 -0800 Subject: [PATCH 3/5] Improve middleware spec with Rack::Lint and cleanup Co-Authored-By: Claude Opus 4.5 --- spec/singed/middleware_spec.rb | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/spec/singed/middleware_spec.rb b/spec/singed/middleware_spec.rb index 93abbcc..0fa2089 100644 --- a/spec/singed/middleware_spec.rb +++ b/spec/singed/middleware_spec.rb @@ -1,39 +1,48 @@ +# frozen_string_literal: true + +require "tmpdir" + describe Singed::RackMiddleware do subject do instance.call(env) end - let(:app) { ->(*) { [200, {'Content-Type' => 'text/plain'}, ['OK']] } } + let(:app) { ->(*) { [200, {"content-type" => "text/plain"}, ["OK"]] } } let(:instance) { described_class.new(app) } - let(:env) { Rack::MockRequest.env_for('/', headers) } + let(:env) { Rack::MockRequest.env_for("/", headers) } let(:headers) { {} } - it 'returns a proper rack response' do - status, _ = subject - expect(status).to eq 200 + before do + allow_any_instance_of(Singed::Flamegraph).to receive(:open) + Singed.output_directory = Dir.mktmpdir("singed-spec") + end + + it "returns a proper rack response" do + linted_app = Rack::Lint.new(instance) + expect { linted_app.call(env) }.not_to raise_error end - it 'does not capture a flamegraph by default' do + it "does not capture a flamegraph by default" do expect(instance).not_to receive(:flamegraph) subject end - context 'when enabled' do + context "when enabled" do before { allow(instance).to receive(:capture_flamegraph?).and_return(true) } - it 'captures a flamegraph' do - expect(instance).to receive(:flamegraph) + it "captures a flamegraph" do + expect(instance).to receive(:flamegraph).and_call_original subject end end - describe '.capture_flamegraph?' do + describe ".capture_flamegraph?" do subject { instance.capture_flamegraph?(env) } it { is_expected.to be false } - context 'when HTTP_X_SINGED is true' do - let(:headers) { { 'HTTP_X_SINGED' => 'true' } } + context "when HTTP_X_SINGED is true" do + let(:headers) { {"HTTP_X_SINGED" => "true"} } it { is_expected.to be true } end From 70419fd336b651a444a82f98f5207e6a254f2519 Mon Sep 17 00:00:00 2001 From: Daniel Pepper Date: Sat, 28 Mar 2026 06:31:45 +0700 Subject: [PATCH 4/5] rack-test not needed --- Gemfile | 1 - Gemfile.lock | 3 --- 2 files changed, 4 deletions(-) diff --git a/Gemfile b/Gemfile index b83ed2e..97879fc 100644 --- a/Gemfile +++ b/Gemfile @@ -6,7 +6,6 @@ source "https://rubygems.org" gemspec gem "activejob" -gem "rack-test" gem "rake", "~> 13.0" gem "rspec" gem "rubyzip" diff --git a/Gemfile.lock b/Gemfile.lock index 04ac319..23aaef0 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -48,8 +48,6 @@ GEM prism (1.9.0) racc (1.8.1) rack (3.2.5) - rack-test (2.2.0) - rack (>= 1.3) rainbow (3.1.1) rake (13.0.6) redis-client (0.26.4) @@ -120,7 +118,6 @@ PLATFORMS DEPENDENCIES activejob - rack-test rake (~> 13.0) rspec rubyzip From ac2c58e8398a377c42d1454eee662fd821a9e1a3 Mon Sep 17 00:00:00 2001 From: Daniel Pepper Date: Sat, 28 Mar 2026 06:45:20 +0700 Subject: [PATCH 5/5] Address PR feedback: sigil fix and cleanup - Change `.capture_flamegraph?` to `#capture_flamegraph?` (instance method) - Remove temp directory setup (now handled globally via spec_helper) Co-Authored-By: Claude Opus 4.5 --- spec/singed/middleware_spec.rb | 60 ++++++++++++++++++++++++++-------- 1 file changed, 47 insertions(+), 13 deletions(-) diff --git a/spec/singed/middleware_spec.rb b/spec/singed/middleware_spec.rb index 0fa2089..0e54183 100644 --- a/spec/singed/middleware_spec.rb +++ b/spec/singed/middleware_spec.rb @@ -1,42 +1,47 @@ # frozen_string_literal: true -require "tmpdir" - describe Singed::RackMiddleware do subject do instance.call(env) end - let(:app) { ->(*) { [200, {"content-type" => "text/plain"}, ["OK"]] } } + let(:app_response) { [200, {"content-type" => "text/plain"}, ["OK"]] } + let(:app) { ->(*) { app_response } } let(:instance) { described_class.new(app) } let(:env) { Rack::MockRequest.env_for("/", headers) } let(:headers) { {} } - before do - allow_any_instance_of(Singed::Flamegraph).to receive(:open) - Singed.output_directory = Dir.mktmpdir("singed-spec") - end - it "returns a proper rack response" do linted_app = Rack::Lint.new(instance) expect { linted_app.call(env) }.not_to raise_error end - it "does not capture a flamegraph by default" do - expect(instance).not_to receive(:flamegraph) - subject + it "passes through the app response unchanged" do + expect(subject).to eq(app_response) end context "when enabled" do - before { allow(instance).to receive(:capture_flamegraph?).and_return(true) } + before do + allow_any_instance_of(Singed::Flamegraph).to receive(:open) + allow(instance).to receive(:capture_flamegraph?).and_return(true) + end it "captures a flamegraph" do expect(instance).to receive(:flamegraph).and_call_original subject end + + it "returns a proper rack response" do + linted_app = Rack::Lint.new(instance) + expect { linted_app.call(env) }.not_to raise_error + end + + it "passes through the app response unchanged" do + expect(subject).to eq(app_response) + end end - describe ".capture_flamegraph?" do + describe "#capture_flamegraph?" do subject { instance.capture_flamegraph?(env) } it { is_expected.to be false } @@ -46,5 +51,34 @@ it { is_expected.to be true } end + + context "when SINGED_MIDDLEWARE_ALWAYS_CAPTURE env var is set" do + around do |example| + original = ENV["SINGED_MIDDLEWARE_ALWAYS_CAPTURE"] + described_class.remove_instance_variable(:@always_capture) if described_class.instance_variable_defined?(:@always_capture) + example.run + ensure + if original.nil? + ENV.delete("SINGED_MIDDLEWARE_ALWAYS_CAPTURE") + else + ENV["SINGED_MIDDLEWARE_ALWAYS_CAPTURE"] = original + end + described_class.remove_instance_variable(:@always_capture) if described_class.instance_variable_defined?(:@always_capture) + end + + %w[true 1 yes].each do |truthy_value| + context "when SINGED_MIDDLEWARE_ALWAYS_CAPTURE=#{truthy_value}" do + before { ENV["SINGED_MIDDLEWARE_ALWAYS_CAPTURE"] = truthy_value } + + it { is_expected.to be true } + end + end + + context "when SINGED_MIDDLEWARE_ALWAYS_CAPTURE=false" do + before { ENV["SINGED_MIDDLEWARE_ALWAYS_CAPTURE"] = "false" } + + it { is_expected.to be false } + end + end end end