[meganemura]

RSpec runs a group's before(:all) hooks once, before the group's first example starts. track_tests records each example, so their lines stayed in the segment still open from the spec file that ran before, or went to no test for the first file. So when code that only such a hook runs was changed, simplecov affected picked that earlier spec file, not the spec file that uses the hook.

When a top-level group starts, its spec file now starts too, before the group's own hooks run (a config-level prepend_before(:context)). What runs from there is credited to the file, as spec/b_spec.rb, the same id that file loading uses.

This matters when some code runs only in before(:all). A common case is records created once per file with test-prof's let_it_be: their model callbacks run there and in no example.

Reproduction

# lib/widget.rb
class Widget
  def quantity?
    n = 1
    n > 0
  end

  def kind
    :round
  end
end

# spec/spec_helper.rb
require "simplecov"
SimpleCov.start do
  track_tests granularity: :file   # also :test
  formatter SimpleCov::Formatter::JSONFormatter
end
require_relative "../lib/widget"

# spec/a_spec.rb
require_relative "spec_helper"
RSpec.describe Widget do
  it { expect(Widget.new.kind).to eq(:round) }
end

# spec/b_before_all_spec.rb
require_relative "spec_helper"
RSpec.describe Widget do
  before(:all) { Widget.new.quantity? }
  it { expect(1).to eq 1 }
end

After rspec --order defined, a commit, and sed -i 's/n = 1$/n = 2/' lib/widget.rb:

Run	main: context / affected	this branch: context / affected
:file, a then b	spec/a_spec.rb / spec/a_spec.rb	spec/b_before_all_spec.rb / same
:file, b alone	none / full suite	spec/b_before_all_spec.rb / same
:test, a then b	spec/a_spec.rb:3 / spec/a_spec.rb	spec/b_before_all_spec.rb / same

Notes

• Cost: at :file the new segment has the id of the file's examples and merges with them, so no extra peek. At :test it is one Coverage.peek_result per top-level group.
• Not changed: a nested group's before(:all) at :test still goes to the previous example of the same file, and after(:all) goes to the file's last example (at :file, to the file). Minitest has no hook outside a test.
• The comment above open_segment said code between two tracks goes to the later one. It goes to the earlier one, which is how this bug happened, so the comment now says that.

Verification

• Sandbox spec: a real rspec run where spec/setting_up_spec.rb calls SetupOnce.call from before(:all). covering(lib/setup_once.rb, 3) was ["spec/meta_magic_spec.rb:14"], from the spec file that ran before, on main and is ["spec/setting_up_spec.rb"] here.
• rake spec, rubocop, rake rbs, steep check, and rake 'mutant:since[origin/main]' (0 alive) pass.

[sferik]

Thanks! Hooks like before(:all) and let_it_be being credited to the previous spec file is a real gap and a boundary before each spec file's setup is the right fix. A few inline comments. The main one is that nested groups still have the problem.

This also touches rspec_example_id's file, and #1319 changes that method, so whichever lands second will need a rebase.

[sferik]

This leaves the same problem in nested groups. Take this spec file:

RSpec.describe Widget do
  it("a") { ... }                    # line 5

  context "when set up" do
    before(:all) { Setup.call }

    it("b") { ... }                  # line 12
  end
end

The nested before(:all) runs after a finishes, outside any test, so at :test granularity Setup's lines go to spec/b_spec.rb:5. A change to Setup then selects only a, which never runs that hook, and b is never selected.

Since the hook runs outside any test either way, could every group mark the boundary, not just top-level ones? It would credit nested setup to the file too. A sandbox case with a nested before(:all) after an example would cover it.

[sferik]

prepend_before only gets ahead of hooks added after it. Two kinds still run first:

1. A config hook added later with config.prepend_before(:all), for example in spec_helper.rb after SimpleCov.start. RSpec unshifts it in front of ours.
2. A group's own prepend_before(:all). RSpec copies the global context hooks into a top-level group when it defines the group, before it evaluates the group body, so a prepend in the body goes in front of them.

Either one runs while the previous segment is still open, so its lines still go to the previous spec file.

ExampleGroup.run calls reporter.example_group_started(group) before any context hook, so a reporter listener on example_group_started would mark the boundary no matter when or where hooks are added. It also fires for nested groups, which would cover the comment above.

[sferik]

This repeats the delete_prefix("./") from rspec_example_id. The two have to produce the same file id, or at :file granularity a file's setup and its examples become two contexts. A shared helper would keep them in step.

Also, the PR description says this is the same id file loading uses. test_file_loading builds it relative to SimpleCov.root, but rerun_file_path is relative to the working directory, so they differ when rspec runs from a subdirectory (an engine, for example). Example ids already have that mismatch, so it isn't new here, but a shared helper would be the place to fix it for both.

[sferik]

This is true only for top-level groups (see the comment on rspec_group_starting). If that stays, could the docs say so? Also, coming right after the JRuby sentence, "too" reads as if it were about JRuby. Starting a new paragraph, or dropping "too", would avoid that.

[sferik]

At :test granularity this adds one Coverage.peek_result per spec file, since the file's segment and its first example have different ids. That happens even in suites with no before(:all) at all. It's one per file, not per example, so the cost is modest, but it might be worth checking whether the group has any before(:context) hooks first, if RSpec exposes that cleanly.

[sferik]

Nit: could this be two sentences instead of a semicolon? "Only a top-level group starts its spec file. A nested group's file already started."

[meganemura]

Thanks for the review. I reworked the change around RSpec's reporter, as you suggested.

1 and 2. Nested groups, and hooks that run before ours: the tracker no longer adds a prepend_before(:context) hook. It registers for the reporter's example_group_started, which RSpec sends for every group, nested ones too, before any of the group's hooks run. Hook order no longer matters. It registers from the before(:suite) hook, because calling config.reporter while spec_helper loads would set up the default formatter before the suite picks its own. The sandbox now has a spec file with a prepend_before(:all) on the top-level group and a before(:all) in a nested group after an example. Both hooks went to the wrong test before this change and go to their spec file now.

3. delete_prefix("./") is now one method, rspec_path, which both the example id and the group's file use. You are right that rerun_file_path is relative to the working directory and not to SimpleCov.root. Example ids from location have the same problem today, so I would like to fix both in a separate PR, if that is fine with you.

4. The docs sentence is now its own paragraph and says that nested groups count too.

5. At :test the change adds one peek per group, and none at :file. On hashie's suite (1072 examples) at :test, peeks go from 1093 to 1451 and the run time is within noise (3.5 to 3.8 s across three runs each). RSpec has no public way to ask whether a group has before(:context) hooks. I tried group.hooks.send(:matching_hooks_for, :before, :context, group). It brings the peeks back to 1093, but it depends on RSpec internals, so I left it out. I can add it if you prefer fewer peeks.

6. The comment with the semicolon is gone with the top-level check.

I squashed the changes into the one commit. #1319 also changes rspec_example_id. Whichever lands first, I will rebase the other one.

[sferik]

Looks good to me. This just needs to be rebased after merging #1319. The conflicts should all be "keep both", with rspec_example_id passing that PR’s id through your new rspec_path.

[meganemura]

Rebased onto main after #1319. The conflicts were all "keep both", and rspec_example_id now passes #1319's id through rspec_path.

[sferik]

Thanks again!