From 1d59a59df683765346fcc6c1fac5da9e3b85b883 Mon Sep 17 00:00:00 2001 From: Daniel Pepper Date: Tue, 19 Nov 2024 06:33:56 +0800 Subject: [PATCH 1/2] rack cleanup --- Gemfile.lock | 3 +++ lib/singed/rack_middleware.rb | 8 ++----- singed.gemspec | 1 + 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.lock b/Gemfile.lock index 7f7dc71..0cb6e40 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -48,6 +48,8 @@ GEM prism (1.9.0) racc (1.7.3) 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) @@ -114,6 +116,7 @@ PLATFORMS DEPENDENCIES activejob + rack-test rake (~> 13.0) rspec sidekiq 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/singed.gemspec b/singed.gemspec index 56936ea..c93ba4a 100644 --- a/singed.gemspec +++ b/singed.gemspec @@ -23,6 +23,7 @@ Gem::Specification.new do |spec| spec.add_dependency "stackprof", ">= 0.2.13" + spec.add_development_dependency "rack-test" spec.add_development_dependency "rake", "~> 13.0" spec.add_development_dependency "rspec" end 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 dcc81ff727dd5025e8f6620503cec408f568899b Mon Sep 17 00:00:00 2001 From: Daniel Pepper Date: Mon, 9 Mar 2026 06:11:06 -0800 Subject: [PATCH 2/2] 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