From e4b354e5719533c5e6dcf99e55ed29358fd798d1 Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Sat, 26 Sep 2026 09:50:49 -0700 Subject: [PATCH 1/4] Replace standardrb with rubocop-gusto Lint with the shared rubocop-gusto configuration used across the other rubyatscale gems instead of Standard. - Gemfile: drop standard, add rubocop-gusto (require: false). The lockfile change is limited to removing standard's gems and adding rubocop-gusto's. sorbet-static (pulled in through rubocop-sorbet) has no pure-Ruby build, so the lockfile now lists concrete platforms instead of `ruby`. - .rubocop.yml: inherit rubocop-gusto's default and sidekiq configs, target Ruby 3.3 (the gemspec minimum), keep RuboCop's default excludes, and disable the Sorbet department because this gem does not use Sorbet. - Remove .standard.yml. - Replace the StandardRB workflow with a RuboCop workflow that runs on push and pull_request with read-only permissions. --- .github/workflows/rubocop.yml | 21 +++++++++ .github/workflows/standardrb.yaml | 15 ------- .rubocop.yml | 33 ++++++++++---- .standard.yml | 2 - Gemfile | 2 +- Gemfile.lock | 73 +++++++++++++++++++++++++------ 6 files changed, 105 insertions(+), 41 deletions(-) create mode 100644 .github/workflows/rubocop.yml delete mode 100644 .github/workflows/standardrb.yaml delete mode 100644 .standard.yml diff --git a/.github/workflows/rubocop.yml b/.github/workflows/rubocop.yml new file mode 100644 index 0000000..655752d --- /dev/null +++ b/.github/workflows/rubocop.yml @@ -0,0 +1,21 @@ +name: RuboCop + +on: [push, pull_request] + +permissions: + contents: read + +jobs: + rubocop: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - name: Set up Ruby + uses: ruby/setup-ruby@95ef2b042f9d7a56d8268cba8559e2842e2ad01b # v1.321.0 + with: + bundler-cache: true + ruby-version: "3.4" + - name: Run RuboCop + run: bundle exec rubocop diff --git a/.github/workflows/standardrb.yaml b/.github/workflows/standardrb.yaml deleted file mode 100644 index f79fbde..0000000 --- a/.github/workflows/standardrb.yaml +++ /dev/null @@ -1,15 +0,0 @@ -name: StandardRB - -on: [push] - -jobs: - standardrb: - runs-on: ubuntu-latest - permissions: - checks: write - contents: write - steps: - - name: StandardRB Linter - uses: standardrb/standard-ruby-action@v1 - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/.rubocop.yml b/.rubocop.yml index da64f40..92f1761 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,12 +1,27 @@ +inherit_mode: + merge: + - Exclude + - Include + +inherit_gem: + rubocop-gusto: + - config/default.yml + - config/sidekiq.yml + +plugins: + - rubocop-gusto + - rubocop-rspec + - rubocop-performance + - rubocop-rake + AllCops: TargetRubyVersion: 3.3 + Exclude: + - 'vendor/**/*' -require: - - standard - - standard-custom - - standard-performance - -inherit_gem: - standard: config/base.yml - standard-custom: config/base.yml - standard-performance: config/base.yml +# singed does not use Sorbet: there is no sorbet/ config, no sorbet runtime dependency, and no +# typecheck step. The Sorbet cops would only add `# typed:` sigils that nothing reads, and +# `# typed: strict` would claim a level of typing this gem does not have. rubocop-gusto's own +# .rubocop.yml disables the department for the same reason. +Sorbet: + Enabled: false diff --git a/.standard.yml b/.standard.yml deleted file mode 100644 index f31800f..0000000 --- a/.standard.yml +++ /dev/null @@ -1,2 +0,0 @@ -fix: true -format: progress diff --git a/Gemfile b/Gemfile index 29ee1a5..5fc3c87 100644 --- a/Gemfile +++ b/Gemfile @@ -8,6 +8,6 @@ gemspec gem "activejob" gem "rake", "~> 13.4" gem "rspec" +gem "rubocop-gusto", require: false gem "rubyzip" gem "sidekiq" -gem "standard" diff --git a/Gemfile.lock b/Gemfile.lock index 171a96d..e755e9d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -26,10 +26,13 @@ GEM ast (2.4.3) base64 (0.3.0) bigdecimal (4.1.2) + code_teams (1.3.1) + sorbet-runtime concurrent-ruby (1.3.8) connection_pool (3.0.2) diff-lcs (1.6.2) drb (2.2.3) + erubi (1.13.1) globalid (1.4.0) activesupport (>= 6.1) i18n (1.15.2) @@ -50,9 +53,17 @@ GEM rack (3.2.7) rainbow (3.1.1) rake (13.4.2) + rbi (0.4.3) + prism (~> 1.0) + rbs (>= 4.0.1) + rbs (4.2.0) + logger + prism (>= 1.6.0) + tsort redis-client (0.30.1) connection_pool regexp_parser (2.12.0) + rexml (3.4.4) rspec (3.13.2) rspec-core (~> 3.13.0) rspec-expectations (~> 3.13.0) @@ -80,10 +91,32 @@ GEM rubocop-ast (1.50.0) parser (>= 3.3.7.2) prism (~> 1.7) + rubocop-gusto (11.9.0) + code_teams + lint_roller + rubocop (>= 1.76) + rubocop-performance + rubocop-rake + rubocop-rspec + rubocop-sorbet (>= 0.13.0) + smart_todo + thor rubocop-performance (1.26.1) lint_roller (~> 1.1) rubocop (>= 1.75.0, < 2.0) rubocop-ast (>= 1.47.1, < 2.0) + rubocop-rake (0.7.1) + lint_roller (~> 1.1) + rubocop (>= 1.72.1) + rubocop-rspec (3.10.2) + lint_roller (~> 1.1) + regexp_parser (>= 2.0) + rubocop (~> 1.86, >= 1.86.2) + rubocop-sorbet (0.16.0) + lint_roller + rbi (~> 0.4) + rubocop (>= 1.75.2) + spoom (~> 1.8) ruby-progressbar (1.13.0) rubyzip (3.5.0) securerandom (0.4.1) @@ -93,19 +126,28 @@ GEM logger (>= 1.7.0) rack (>= 3.2.0) redis-client (>= 0.29.0) + smart_todo (1.11.0) + prism (~> 1.0) + sorbet (0.6.13508) + sorbet-static (= 0.6.13508) + sorbet-runtime (0.6.13508) + sorbet-static (0.6.13508-aarch64-linux) + sorbet-static (0.6.13508-universal-darwin) + sorbet-static (0.6.13508-x86_64-linux) + sorbet-static-and-runtime (0.6.13508) + sorbet (= 0.6.13508) + sorbet-runtime (= 0.6.13508) + spoom (1.8.9) + erubi (>= 1.10.0) + prism (>= 0.28.0) + rbi (>= 0.4.2) + rbs (>= 4.0.0.dev.5) + rexml (>= 3.2.6) + sorbet-static-and-runtime (>= 0.5.10187) + thor (>= 0.19.2) stackprof (0.2.28) - standard (1.56.0) - language_server-protocol (~> 3.17.0.2) - lint_roller (~> 1.0) - rubocop (~> 1.88.0) - standard-custom (~> 1.0.0) - standard-performance (~> 1.8) - standard-custom (1.0.2) - lint_roller (~> 1.0) - rubocop (~> 1.50) - standard-performance (1.9.0) - lint_roller (~> 1.1) - rubocop-performance (~> 1.26.0) + thor (1.5.0) + tsort (0.2.0) tzinfo (2.0.6) concurrent-ruby (~> 1.0) unicode-display_width (3.2.0) @@ -114,16 +156,19 @@ GEM uri (1.1.1) PLATFORMS - ruby + aarch64-linux + arm64-darwin + x86_64-darwin + x86_64-linux DEPENDENCIES activejob rake (~> 13.4) rspec + rubocop-gusto rubyzip sidekiq singed! - standard BUNDLED WITH 4.0.15 From 9eb0e182510a7860c4d63e1b06be534ba558faa6 Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Sat, 26 Sep 2026 09:56:17 -0700 Subject: [PATCH 2/4] Apply rubocop-gusto autocorrections Safe autocorrections (rubocop -a): shorthand hash values, hash brace spacing, trailing commas in multiline literals, percent-literal delimiters, the `->` lambda literal, block delimiters, symbol-style RSpec metadata, the unused Sidekiq middleware `queue` argument renamed to `_queue`, and removal of disable directives for cops that are not loaded (Rails/TimeZone) or not enabled (Metrics/AbcSize), along with the matching orphaned `rubocop:enable`. Unsafe autocorrections, applied one cop at a time and reviewed: - Style/FrozenStringLiteralComment: add the magic comment to the 13 files that lacked it, including exe/singed, plus the blank line after it that Layout/EmptyLineAfterMagicComment requires. - Gusto/RedundantSpecHelperRequire: drop `require "spec_helper"` from the Sidekiq spec; .rspec already passes `--require spec_helper`. - Style/HashEachMethods: `list.each { |_addr, frame| }` becomes `list.each_value { |frame| }` in Singed::Report#filter!. StackProf::Report#frames always returns a Hash. --- Rakefile | 2 +- bin/rspec | 6 ++++-- exe/singed | 1 + lib/singed.rb | 2 +- lib/singed/backtrace_cleaner_ext.rb | 2 ++ lib/singed/cli.rb | 14 ++++++++------ lib/singed/controller_ext.rb | 4 +++- lib/singed/flamegraph.rb | 10 ++++++---- lib/singed/kernel_ext.rb | 4 +++- lib/singed/rack_middleware.rb | 2 ++ lib/singed/railtie.rb | 2 ++ lib/singed/report.rb | 4 +++- lib/singed/rspec.rb | 2 ++ lib/singed/sidekiq.rb | 4 ++-- singed.gemspec | 4 ++-- spec/singed/kernel_ext_spec.rb | 21 +++++++++++---------- spec/singed/middleware_spec.rb | 8 ++++---- spec/singed/sidekiq_spec.rb | 7 +++---- spec/singed/speedscope_spec.rb | 2 +- spec/spec_helper.rb | 2 ++ spec/support/sidekiq.rb | 4 ++-- 21 files changed, 65 insertions(+), 42 deletions(-) diff --git a/Rakefile b/Rakefile index 638ee1b..d45be73 100644 --- a/Rakefile +++ b/Rakefile @@ -52,4 +52,4 @@ end Rake::Task[:build].enhance ["speedscope:vendor"] Rake::Task[:clobber].enhance ["speedscope:clobber"] -task default: %i[] +task default: %i() diff --git a/bin/rspec b/bin/rspec index cb53ebe..3fd48d8 100755 --- a/bin/rspec +++ b/bin/rspec @@ -16,8 +16,10 @@ if File.file?(bundle_binstub) if File.read(bundle_binstub, 300).include?("This file was generated by Bundler") load(bundle_binstub) else - abort("Your `bin/bundle` was not generated by Bundler, so this binstub cannot run. -Replace `bin/bundle` by running `bundle binstubs bundler --force`, then run this command again.") + abort( + "Your `bin/bundle` was not generated by Bundler, so this binstub cannot run. +Replace `bin/bundle` by running `bundle binstubs bundler --force`, then run this command again." + ) end end diff --git a/exe/singed b/exe/singed index 3fc6b7c..4cf651a 100755 --- a/exe/singed +++ b/exe/singed @@ -1,4 +1,5 @@ #!/usr/bin/env ruby +# frozen_string_literal: true require "singed/cli" if Singed::CLI.chdir_rails_root diff --git a/lib/singed.rb b/lib/singed.rb index f2a48af..69c2e58 100644 --- a/lib/singed.rb +++ b/lib/singed.rb @@ -49,7 +49,7 @@ def start(label = nil, ignore_gc: false, interval: 1000) return unless enabled? return if profiling? - @current_flamegraph = Flamegraph.new(label: label, ignore_gc: ignore_gc, interval: interval) + @current_flamegraph = Flamegraph.new(label:, ignore_gc:, interval:) @current_flamegraph.tap(&:start) end diff --git a/lib/singed/backtrace_cleaner_ext.rb b/lib/singed/backtrace_cleaner_ext.rb index 0042fed..52ae9a3 100644 --- a/lib/singed/backtrace_cleaner_ext.rb +++ b/lib/singed/backtrace_cleaner_ext.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ActiveSupport class BacktraceCleaner def filter_line(line) diff --git a/lib/singed/cli.rb b/lib/singed/cli.rb index 7976106..17c3232 100644 --- a/lib/singed/cli.rb +++ b/lib/singed/cli.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require "shellwords" require "tmpdir" require "optionparser" @@ -75,14 +77,14 @@ def run format: "speedscope", file: filename.to_s, rate: @rate, - silent: nil + silent: nil, } rbspy_args = [ "record", *options.map { |k, v| ["--#{k}", v].compact }.flatten, "--", - *argv + *argv, ] loop do @@ -92,9 +94,9 @@ def run prompt_password end - rbspy = lambda do + rbspy = -> do # don't run things with spring, because it forks and rbspy won't see it - sudo ["rbspy", *rbspy_args], reason: "Singed needs to run as root, but will drop permissions back to your user.", env: {"DISABLE_SPRING" => "1"} + sudo ["rbspy", *rbspy_args], reason: "Singed needs to run as root, but will drop permissions back to your user.", env: { "DISABLE_SPRING" => "1" } end if defined?(Bundler) @@ -122,7 +124,7 @@ def run end filename.write(JSON.dump(json)) - flamegraph = Singed::Flamegraph.new(filename: filename) + flamegraph = Singed::Flamegraph.new(filename:) flamegraph.open end @@ -153,7 +155,7 @@ def sudo(system_args, reason:, env: {}) sudo_args = [ "sudo", "--preserve-env", - *system_args.map(&:to_s) + *system_args.map(&:to_s), ] puts "$ #{Shellwords.join(sudo_args)}" diff --git a/lib/singed/controller_ext.rb b/lib/singed/controller_ext.rb index f630fb5..360a0e1 100644 --- a/lib/singed/controller_ext.rb +++ b/lib/singed/controller_ext.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Singed module ControllerExt def self.included(base) @@ -8,7 +10,7 @@ module ClassMethods # Define an around_action to generate flamegraph for a controller action. def flamegraph(target_action, ignore_gc: false, interval: 1000) around_action(only: target_action) do |controller, action| - controller.flamegraph(ignore_gc: ignore_gc, interval: interval, &action) + controller.flamegraph(ignore_gc:, interval:, &action) end end end diff --git a/lib/singed/flamegraph.rb b/lib/singed/flamegraph.rb index d5d78ec..3f79bfa 100644 --- a/lib/singed/flamegraph.rb +++ b/lib/singed/flamegraph.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Singed class Flamegraph attr_accessor :profile, :filename @@ -17,8 +19,8 @@ def initialize(label: nil, ignore_gc: false, interval: 1000, filename: nil) else @ignore_gc = ignore_gc @interval = interval - @time = Time.now # rubocop:disable Rails/TimeZone - @filename = self.class.generate_filename(label: label, time: @time) + @time = Time.now + @filename = self.class.generate_filename(label:, time: @time) end end @@ -69,11 +71,11 @@ def open_command Singed::Speedscope.open_command(@filename) end - def self.generate_filename(label: nil, time: Time.now) # rubocop:disable Rails/TimeZone + def self.generate_filename(label: nil, time: Time.now) formatted_time = time.strftime("%Y%m%d%H%M%S-%6N") basename_parts = ["speedscope", label, formatted_time].compact - file = Singed.output_directory.join("#{basename_parts.join("-")}.json") + file = Singed.output_directory.join("#{basename_parts.join('-')}.json") # convert to relative directory if it's an absolute path and within the current pwd = Pathname.pwd file = file.relative_path_from(pwd) if file.absolute? && file.to_s.start_with?(pwd.to_s) diff --git a/lib/singed/kernel_ext.rb b/lib/singed/kernel_ext.rb index c566b92..e1cb741 100644 --- a/lib/singed/kernel_ext.rb +++ b/lib/singed/kernel_ext.rb @@ -1,6 +1,8 @@ +# frozen_string_literal: true + module Kernel def flamegraph(label = nil, open: true, ignore_gc: false, interval: 1000, io: $stdout, &block) - fg = Singed::Flamegraph.new(label: label, ignore_gc: ignore_gc, interval: interval) + fg = Singed::Flamegraph.new(label:, ignore_gc:, interval:) result = fg.record(&block) fg.save diff --git a/lib/singed/rack_middleware.rb b/lib/singed/rack_middleware.rb index f267974..d497e62 100644 --- a/lib/singed/rack_middleware.rb +++ b/lib/singed/rack_middleware.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # Rack Middleware require "rack" diff --git a/lib/singed/railtie.rb b/lib/singed/railtie.rb index 5fae6e0..45a5ea6 100644 --- a/lib/singed/railtie.rb +++ b/lib/singed/railtie.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require "singed/backtrace_cleaner_ext" require "singed/controller_ext" diff --git a/lib/singed/report.rb b/lib/singed/report.rb index cbf2753..53b90dd 100644 --- a/lib/singed/report.rb +++ b/lib/singed/report.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Singed class Report < StackProf::Report def filter! @@ -27,7 +29,7 @@ def filter! # list.each{ |_addr, frame| frame[:edges]&.delete_if{ |k,v| list[k].nil? } } # end copy-pasted section - list.each do |_addr, frame| + list.each_value do |frame| frame[:file] = Singed.filter_line(frame[:file]) end diff --git a/lib/singed/rspec.rb b/lib/singed/rspec.rb index 16aee70..4978bde 100644 --- a/lib/singed/rspec.rb +++ b/lib/singed/rspec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require "singed" RSpec.configure do |config| diff --git a/lib/singed/sidekiq.rb b/lib/singed/sidekiq.rb index ac478ba..29a4165 100644 --- a/lib/singed/sidekiq.rb +++ b/lib/singed/sidekiq.rb @@ -5,7 +5,7 @@ module Sidekiq class ServerMiddleware include ::Sidekiq::ServerMiddleware - def call(job_instance, job_payload, queue, &block) + def call(job_instance, job_payload, _queue, &block) return block.call unless capture_flamegraph?(job_instance, job_payload) flamegraph(flamegraph_label(job_instance, job_payload), &block) @@ -13,7 +13,7 @@ def call(job_instance, job_payload, queue, &block) private - TRUTHY_STRINGS = %w[true 1 yes].freeze + TRUTHY_STRINGS = %w(true 1 yes).freeze def capture_flamegraph?(job_instance, job_payload) return TRUTHY_STRINGS.include?(job_payload["x-singed"].to_s) if job_payload.key?("x-singed") diff --git a/singed.gemspec b/singed.gemspec index 8895069..050e805 100644 --- a/singed.gemspec +++ b/singed.gemspec @@ -13,12 +13,12 @@ Gem::Specification.new do |spec| spec.metadata = { "source_code_uri" => "https://github.com/rubyatscale/singed", "bug_tracker_uri" => "https://github.com/rubyatscale/singed/issues", - "homepage_uri" => "https://github.com/rubyatscale/singed" + "homepage_uri" => "https://github.com/rubyatscale/singed", } spec.files = Dir["README.md", "*.gemspec", "lib/**/*", "exe/**/*", "vendor/speedscope/**/*"] spec.bindir = "exe" - spec.executables = spec.files.grep(%r{\Aexe/}) { |f| File.basename(f) } + spec.executables = spec.files.grep(%r(\Aexe/)) { |f| File.basename(f) } spec.require_paths = ["lib"] spec.add_dependency "stackprof", ">= 0.2.13" diff --git a/spec/singed/kernel_ext_spec.rb b/spec/singed/kernel_ext_spec.rb index 98d8552..3acc78f 100644 --- a/spec/singed/kernel_ext_spec.rb +++ b/spec/singed/kernel_ext_spec.rb @@ -1,7 +1,10 @@ +# frozen_string_literal: true + describe Kernel, "extension" do - let(:flamegraph) { + let(:flamegraph) do instance_double(Singed::Flamegraph) - } + end + let(:io) { StringIO.new } before do allow(Singed::Flamegraph).to receive(:new).and_return(flamegraph) @@ -12,20 +15,18 @@ allow(flamegraph).to receive(:filename) end - let(:io) { StringIO.new } - it "works without any arguments" do # * except what's needed to test # note: use Object.new to get the actual flamegraph kernel extension, instead of the rspec-specific flamegraph - Object.new.flamegraph io: io do + Object.new.flamegraph(io:) do end expect(Singed::Flamegraph).to have_received(:new).with(label: nil, ignore_gc: false, interval: 1000) end it "works with explicit arguments" do - # note: use Object.new to get the actual flamegraph kernel extension, instead of the rspec-specific flamegraph - Object.new.flamegraph "yellowjackets", ignore_gc: true, interval: 2000, io: io do + # NOTE: use Object.new to get the actual flamegraph kernel extension, instead of the rspec-specific flamegraph + Object.new.flamegraph("yellowjackets", ignore_gc: true, interval: 2000, io:) do end expect(Singed::Flamegraph).to have_received(:new).with(label: "yellowjackets", ignore_gc: true, interval: 2000) @@ -33,7 +34,7 @@ context "default" do it "opens" do - Object.new.flamegraph open: true, io: io do + Object.new.flamegraph(open: true, io:) do end expect(flamegraph).to have_received(:open) @@ -42,7 +43,7 @@ context "open: true" do it "opens" do - Object.new.flamegraph open: true, io: io do + Object.new.flamegraph(open: true, io:) do end expect(flamegraph).to have_received(:open) @@ -51,7 +52,7 @@ context "open: false" do it "doesn't open" do - Object.new.flamegraph open: false, io: io do + Object.new.flamegraph(open: false, io:) do end expect(flamegraph).to_not have_received(:open) diff --git a/spec/singed/middleware_spec.rb b/spec/singed/middleware_spec.rb index 0e54183..09b6612 100644 --- a/spec/singed/middleware_spec.rb +++ b/spec/singed/middleware_spec.rb @@ -5,8 +5,8 @@ instance.call(env) end - let(:app_response) { [200, {"content-type" => "text/plain"}, ["OK"]] } - let(:app) { ->(*) { app_response } } + 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) { {} } @@ -47,7 +47,7 @@ it { is_expected.to be false } context "when HTTP_X_SINGED is true" do - let(:headers) { {"HTTP_X_SINGED" => "true"} } + let(:headers) { { "HTTP_X_SINGED" => "true" } } it { is_expected.to be true } end @@ -66,7 +66,7 @@ described_class.remove_instance_variable(:@always_capture) if described_class.instance_variable_defined?(:@always_capture) end - %w[true 1 yes].each do |truthy_value| + %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 } diff --git a/spec/singed/sidekiq_spec.rb b/spec/singed/sidekiq_spec.rb index 72be9ae..4fc04ef 100644 --- a/spec/singed/sidekiq_spec.rb +++ b/spec/singed/sidekiq_spec.rb @@ -1,12 +1,11 @@ # frozen_string_literal: true -require "spec_helper" require "sidekiq" require "active_job" require "singed/sidekiq" require_relative "../support/sidekiq" -RSpec.describe Singed::Sidekiq::ServerMiddleware, sidekiq: true do +RSpec.describe Singed::Sidekiq::ServerMiddleware, :sidekiq do subject { job_class.set(job_modifiers).perform_async(*job_args) } let(:job_class) { SidekiqPlainJob } @@ -26,7 +25,7 @@ end context "when x-singed payload is true" do - let(:job_modifiers) { {"x-singed" => true} } + let(:job_modifiers) { { "x-singed" => true } } it "wraps execution in flamegraph when x-singed is true" do expect_any_instance_of(described_class).to receive(:flamegraph) @@ -46,7 +45,7 @@ end context "when payload satisfies capture_flamegraph?" do - let(:job_modifiers) { {"x-flamegraph" => true} } + let(:job_modifiers) { { "x-flamegraph" => true } } it "wraps execution in flamegraph when capture_flamegraph? returns true" do expect_any_instance_of(described_class).to receive(:flamegraph) diff --git a/spec/singed/speedscope_spec.rb b/spec/singed/speedscope_spec.rb index ee292a8..9820ce5 100644 --- a/spec/singed/speedscope_spec.rb +++ b/spec/singed/speedscope_spec.rb @@ -21,7 +21,7 @@ described_class.open(profile_path) - expect(described_class).to have_received(:system).with(described_class.send(:os_open_command), %r{\Afile://}) + expect(described_class).to have_received(:system).with(described_class.send(:os_open_command), %r(\Afile://)) end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 72cb8f2..4eeeb21 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # This file was generated by the `rspec --init` command. Conventionally, all # specs live under a `spec` directory, which RSpec adds to the `$LOAD_PATH`. # The generated `.rspec` file contains `--require spec_helper` which will cause diff --git a/spec/support/sidekiq.rb b/spec/support/sidekiq.rb index c813758..b386b58 100644 --- a/spec/support/sidekiq.rb +++ b/spec/support/sidekiq.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require "singed/sidekiq" RSpec.configure do |config| @@ -19,7 +21,6 @@ # Sidekiq doesn't invoke middlewares in inline testingmode, so we need to invoke it oursleves module SidekiqTestingInlineWithMiddlewares - # rubocop:disable Metrics/AbcSize def push(job) return super unless Sidekiq::Testing.inline? @@ -34,7 +35,6 @@ def push(job) end job["jid"] end - # rubocop:enable Metrics/AbcSize end class SidekiqPlainJob From 7703de6b8b63b7f46b098e3a6aca50825d02153f Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Sat, 26 Sep 2026 10:03:38 -0700 Subject: [PATCH 3/4] Fix remaining rubocop-gusto offenses by hand Library: - Singed::ControllerExt: use ActiveSupport::Concern instead of a hand-written `self.included` hook (Gusto/NoMetaprogramming). Including the module still extends the host with ClassMethods, and Singed::ControllerExt::ClassMethods is unchanged. A new spec covers this. - Singed::Sidekiq::ServerMiddleware: move TRUTHY_STRINGS above `private`, which never applied to it (Lint/UselessConstantScoping). The constant stays public, as it was before. - Singed: keep `extend self` with an inline Style/ModuleFunction disable. `extend self` makes every Singed method a public instance method of the module as well as a module method. Moving them into `class << self` would remove those instance methods for anyone who includes or extends Singed. Specs: - Replace allow_/expect_any_instance_of (RSpec/AnyInstance). The Rack middleware spec stubs Singed::Speedscope.open. The Sidekiq spec hands Sidekiq and ActiveJob the middleware and job instances it asserts against. Breaking the middleware on purpose (always profiling, never profiling, or dropping the job) still fails the suite. - Rename middleware_spec.rb and sidekiq_spec.rb to match the classes they describe (RSpec/SpecFilePathFormat), and drop the "extension" suffix from the Kernel spec's describe. - Reword contexts to start with when/with (RSpec/ContextWording). The "default" Kernel#flamegraph context now omits `open:` so it actually tests the default, rather than repeating the `open: true` context (RSpec/RepeatedExampleGroupBody). - Call the private Speedscope.os_open_command with __send__ (Style/Send). Adding the frozen_string_literal comment everywhere caused no FrozenError. A review of every mutating call in lib/, exe/, bin/ and the Rakefile found no string literal that is mutated after it is created. --- lib/singed.rb | 5 +- lib/singed/controller_ext.rb | 6 +- lib/singed/sidekiq.rb | 4 +- spec/singed/controller_ext_spec.rb | 40 +++++++++++++ spec/singed/kernel_ext_spec.rb | 12 ++-- ...leware_spec.rb => rack_middleware_spec.rb} | 2 +- .../server_middleware_spec.rb} | 56 ++++++++++--------- spec/singed/speedscope_spec.rb | 16 +++--- 8 files changed, 95 insertions(+), 46 deletions(-) create mode 100644 spec/singed/controller_ext_spec.rb rename spec/singed/{middleware_spec.rb => rack_middleware_spec.rb} (97%) rename spec/singed/{sidekiq_spec.rb => sidekiq/server_middleware_spec.rb} (66%) diff --git a/lib/singed.rb b/lib/singed.rb index 69c2e58..5743c37 100644 --- a/lib/singed.rb +++ b/lib/singed.rb @@ -4,7 +4,10 @@ require "stackprof" module Singed - extend self + # Every method below is both a module method (Singed.start) and a public instance method of + # Singed, which is how the gem has shipped since its first release. `class << self` would + # remove the instance methods for anyone who includes or extends Singed. + extend self # rubocop:disable Style/ModuleFunction # Where should flamegraphs be saved? def output_directory=(directory) diff --git a/lib/singed/controller_ext.rb b/lib/singed/controller_ext.rb index 360a0e1..2e51b0f 100644 --- a/lib/singed/controller_ext.rb +++ b/lib/singed/controller_ext.rb @@ -1,10 +1,10 @@ # frozen_string_literal: true +require "active_support/concern" + module Singed module ControllerExt - def self.included(base) - base.extend(ClassMethods) - end + extend ActiveSupport::Concern module ClassMethods # Define an around_action to generate flamegraph for a controller action. diff --git a/lib/singed/sidekiq.rb b/lib/singed/sidekiq.rb index 29a4165..e65f15f 100644 --- a/lib/singed/sidekiq.rb +++ b/lib/singed/sidekiq.rb @@ -5,6 +5,8 @@ module Sidekiq class ServerMiddleware include ::Sidekiq::ServerMiddleware + TRUTHY_STRINGS = %w(true 1 yes).freeze + def call(job_instance, job_payload, _queue, &block) return block.call unless capture_flamegraph?(job_instance, job_payload) @@ -13,8 +15,6 @@ def call(job_instance, job_payload, _queue, &block) private - TRUTHY_STRINGS = %w(true 1 yes).freeze - def capture_flamegraph?(job_instance, job_payload) return TRUTHY_STRINGS.include?(job_payload["x-singed"].to_s) if job_payload.key?("x-singed") diff --git a/spec/singed/controller_ext_spec.rb b/spec/singed/controller_ext_spec.rb new file mode 100644 index 0000000..b7c9522 --- /dev/null +++ b/spec/singed/controller_ext_spec.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +require "singed/controller_ext" + +RSpec.describe Singed::ControllerExt do + let(:controller_class) do + Class.new do + def self.around_action(**options, &block) + around_actions << [options, block] + end + + def self.around_actions + @around_actions ||= [] + end + + include Singed::ControllerExt + end + end + + it "adds the flamegraph class method when included" do + expect(controller_class).to respond_to(:flamegraph) + end + + it "wraps the target action in a flamegraph" do + controller_class.flamegraph(:show, ignore_gc: true, interval: 500) + + expect(controller_class.around_actions.size).to eq(1) + options, callback = controller_class.around_actions.first + expect(options).to eq(only: :show) + + controller = controller_class.new + allow(controller).to receive(:flamegraph) { |**, &action| action.call } + action_ran = false + + callback.call(controller, -> { action_ran = true }) + + expect(controller).to have_received(:flamegraph).with(ignore_gc: true, interval: 500) + expect(action_ran).to be(true) + end +end diff --git a/spec/singed/kernel_ext_spec.rb b/spec/singed/kernel_ext_spec.rb index 3acc78f..5d48999 100644 --- a/spec/singed/kernel_ext_spec.rb +++ b/spec/singed/kernel_ext_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -describe Kernel, "extension" do +describe Kernel do let(:flamegraph) do instance_double(Singed::Flamegraph) end @@ -17,7 +17,7 @@ it "works without any arguments" do # * except what's needed to test - # note: use Object.new to get the actual flamegraph kernel extension, instead of the rspec-specific flamegraph + # NOTE: use Object.new to get the actual flamegraph kernel extension, instead of the rspec-specific flamegraph Object.new.flamegraph(io:) do end @@ -32,16 +32,16 @@ expect(Singed::Flamegraph).to have_received(:new).with(label: "yellowjackets", ignore_gc: true, interval: 2000) end - context "default" do + context "with default options" do it "opens" do - Object.new.flamegraph(open: true, io:) do + Object.new.flamegraph(io:) do end expect(flamegraph).to have_received(:open) end end - context "open: true" do + context "with open: true" do it "opens" do Object.new.flamegraph(open: true, io:) do end @@ -50,7 +50,7 @@ end end - context "open: false" do + context "with open: false" do it "doesn't open" do Object.new.flamegraph(open: false, io:) do end diff --git a/spec/singed/middleware_spec.rb b/spec/singed/rack_middleware_spec.rb similarity index 97% rename from spec/singed/middleware_spec.rb rename to spec/singed/rack_middleware_spec.rb index 09b6612..037aa62 100644 --- a/spec/singed/middleware_spec.rb +++ b/spec/singed/rack_middleware_spec.rb @@ -22,7 +22,7 @@ context "when enabled" do before do - allow_any_instance_of(Singed::Flamegraph).to receive(:open) + allow(Singed::Speedscope).to receive(:open) allow(instance).to receive(:capture_flamegraph?).and_return(true) end diff --git a/spec/singed/sidekiq_spec.rb b/spec/singed/sidekiq/server_middleware_spec.rb similarity index 66% rename from spec/singed/sidekiq_spec.rb rename to spec/singed/sidekiq/server_middleware_spec.rb index 4fc04ef..eb93a13 100644 --- a/spec/singed/sidekiq_spec.rb +++ b/spec/singed/sidekiq/server_middleware_spec.rb @@ -3,7 +3,7 @@ require "sidekiq" require "active_job" require "singed/sidekiq" -require_relative "../support/sidekiq" +require_relative "../../support/sidekiq" RSpec.describe Singed::Sidekiq::ServerMiddleware, :sidekiq do subject { job_class.set(job_modifiers).perform_async(*job_args) } @@ -11,16 +11,22 @@ let(:job_class) { SidekiqPlainJob } let(:job_args) { [] } let(:job_modifiers) { {} } + let(:middleware) { described_class.new } + let(:job) { job_class.new } before do - allow_any_instance_of(described_class).to receive(:flamegraph) { |*, &block| block.call } - allow_any_instance_of(job_class).to receive(:perform).and_call_original + # Sidekiq builds a new middleware instance per job, and both Sidekiq and ActiveJob build the + # job instance themselves, so hand them the instances the examples assert against. + allow(described_class).to receive(:new).and_return(middleware) + allow(middleware).to receive(:flamegraph) { |*, &block| block.call } + allow(job_class).to receive(:new).and_return(job) + allow(job).to receive(:perform).and_call_original end context "with plain Sidekiq jobs" do it "doesn't capture flamegraph by default" do - expect_any_instance_of(described_class).not_to receive(:flamegraph) - expect_any_instance_of(job_class).to receive(:perform) + expect(middleware).not_to receive(:flamegraph) + expect(job).to receive(:perform) subject end @@ -28,8 +34,8 @@ let(:job_modifiers) { { "x-singed" => true } } it "wraps execution in flamegraph when x-singed is true" do - expect_any_instance_of(described_class).to receive(:flamegraph) - expect_any_instance_of(job_class).to receive(:perform) + expect(middleware).to receive(:flamegraph) + expect(job).to receive(:perform) subject end end @@ -39,8 +45,8 @@ let(:job_class) { SidekiqFlamegraphJob } it "doesn't capture when capture_flamegraph? returns false" do - expect_any_instance_of(described_class).not_to receive(:flamegraph) - expect_any_instance_of(job_class).to receive(:perform) + expect(middleware).not_to receive(:flamegraph) + expect(job).to receive(:perform) subject end @@ -48,8 +54,8 @@ let(:job_modifiers) { { "x-flamegraph" => true } } it "wraps execution in flamegraph when capture_flamegraph? returns true" do - expect_any_instance_of(described_class).to receive(:flamegraph) - expect_any_instance_of(job_class).to receive(:perform) + expect(middleware).to receive(:flamegraph) + expect(job).to receive(:perform) subject end end @@ -71,8 +77,8 @@ before { ENV["SINGED_MIDDLEWARE_ALWAYS_CAPTURE"] = "true" } it "wraps execution in flamegraph" do - expect_any_instance_of(described_class).to receive(:flamegraph) - expect_any_instance_of(job_class).to receive(:perform) + expect(middleware).to receive(:flamegraph) + expect(job).to receive(:perform) subject end end @@ -81,8 +87,8 @@ before { ENV["SINGED_MIDDLEWARE_ALWAYS_CAPTURE"] = "false" } it "doesn't capture flamegraph" do - expect_any_instance_of(described_class).not_to receive(:flamegraph) - expect_any_instance_of(job_class).to receive(:perform) + expect(middleware).not_to receive(:flamegraph) + expect(job).to receive(:perform) subject end end @@ -95,8 +101,8 @@ let(:job_class) { ActiveJobPlainJob } it "doesn't capture flamegraph by default" do - expect_any_instance_of(described_class).not_to receive(:flamegraph) - expect_any_instance_of(job_class).to receive(:perform) + expect(middleware).not_to receive(:flamegraph) + expect(job).to receive(:perform) subject end @@ -104,8 +110,8 @@ let(:job_class) { ActiveJobFlamegraphJob } it "wraps execution in flamegraph when capture_flamegraph? returns true" do - expect_any_instance_of(described_class).to receive(:flamegraph) - expect_any_instance_of(job_class).to receive(:perform) + expect(middleware).to receive(:flamegraph) + expect(job).to receive(:perform) subject end end @@ -114,8 +120,8 @@ let(:job_class) { ActiveJobNoFlamegraphJob } it "doesn't capture when capture_flamegraph? returns false" do - expect_any_instance_of(described_class).not_to receive(:flamegraph) - expect_any_instance_of(job_class).to receive(:perform) + expect(middleware).not_to receive(:flamegraph) + expect(job).to receive(:perform) subject end end @@ -136,8 +142,8 @@ before { ENV["SINGED_MIDDLEWARE_ALWAYS_CAPTURE"] = "true" } it "wraps execution in flamegraph" do - expect_any_instance_of(described_class).to receive(:flamegraph) - expect_any_instance_of(job_class).to receive(:perform) + expect(middleware).to receive(:flamegraph) + expect(job).to receive(:perform) subject end end @@ -146,8 +152,8 @@ before { ENV["SINGED_MIDDLEWARE_ALWAYS_CAPTURE"] = "false" } it "doesn't capture flamegraph" do - expect_any_instance_of(described_class).not_to receive(:flamegraph) - expect_any_instance_of(job_class).to receive(:perform) + expect(middleware).not_to receive(:flamegraph) + expect(job).to receive(:perform) subject end end diff --git a/spec/singed/speedscope_spec.rb b/spec/singed/speedscope_spec.rb index 9820ce5..24665d3 100644 --- a/spec/singed/speedscope_spec.rb +++ b/spec/singed/speedscope_spec.rb @@ -21,7 +21,7 @@ described_class.open(profile_path) - expect(described_class).to have_received(:system).with(described_class.send(:os_open_command), %r(\Afile://)) + expect(described_class).to have_received(:system).with(described_class.__send__(:os_open_command), %r(\Afile://)) end end @@ -42,36 +42,36 @@ describe ".os_open_command" do it "returns a command and does not raise" do - expect { described_class.send(:os_open_command) }.not_to raise_error - expect(described_class.send(:os_open_command)).to match(/\A(start|open|xdg-open)\z/) + expect { described_class.__send__(:os_open_command) }.not_to raise_error + expect(described_class.__send__(:os_open_command)).to match(/\A(start|open|xdg-open)\z/) end context "when host_os is stubbed" do - subject { described_class.send(:os_open_command) } + subject { described_class.__send__(:os_open_command) } before do allow(RbConfig::CONFIG).to receive(:[]).with("host_os").and_return(stubbed_os) end - context "on Windows" do + context "when running on Windows" do let(:stubbed_os) { "mingw32" } it { is_expected.to eq("start") } end - context "on MacOS" do + context "when running on macOS" do let(:stubbed_os) { "darwin22.0" } it { is_expected.to eq("open") } end - context "on Linux" do + context "when running on Linux" do let(:stubbed_os) { "linux-gnu" } it { is_expected.to eq("xdg-open") } end - context "on unsupported OS" do + context "when running on an unsupported OS" do let(:stubbed_os) { "unknown-os" } it "raises error" do From 9b44e8515602745b71b4a5c4a72fa42cc2a3e6e5 Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Sat, 26 Sep 2026 10:23:36 -0700 Subject: [PATCH 4/4] Tighten ControllerExt spec and extend self comment The "adds the flamegraph class method" example asserted respond_to(:flamegraph), which every object satisfies because Kernel#flamegraph is public. It now checks that the method is owned by ControllerExt::ClassMethods, so it fails if the Concern stops extending ClassMethods. The comment on `extend self` claimed every method is also an instance method; `self.output_directory` is singleton-only, so say plain `def` methods instead. --- lib/singed.rb | 6 +++--- spec/singed/controller_ext_spec.rb | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/singed.rb b/lib/singed.rb index 5743c37..a3dabc7 100644 --- a/lib/singed.rb +++ b/lib/singed.rb @@ -4,9 +4,9 @@ require "stackprof" module Singed - # Every method below is both a module method (Singed.start) and a public instance method of - # Singed, which is how the gem has shipped since its first release. `class << self` would - # remove the instance methods for anyone who includes or extends Singed. + # Methods defined with plain `def` below are both module methods (Singed.start) and public + # instance methods of Singed, which is how the gem has shipped since its first release. + # `class << self` would remove those instance methods for anyone who includes or extends Singed. extend self # rubocop:disable Style/ModuleFunction # Where should flamegraphs be saved? diff --git a/spec/singed/controller_ext_spec.rb b/spec/singed/controller_ext_spec.rb index b7c9522..c7a3672 100644 --- a/spec/singed/controller_ext_spec.rb +++ b/spec/singed/controller_ext_spec.rb @@ -17,8 +17,9 @@ def self.around_actions end end + # Kernel#flamegraph makes every object respond to :flamegraph, so check where the method comes from. it "adds the flamegraph class method when included" do - expect(controller_class).to respond_to(:flamegraph) + expect(controller_class.method(:flamegraph).owner).to eq(Singed::ControllerExt::ClassMethods) end it "wraps the target action in a flamegraph" do