Skip to content

Reset default download queue between specs - #23424

Open
MikeMcQuaid wants to merge 1 commit into
mainfrom
fix-rspec-mocks-lifecycle
Open

Reset default download queue between specs#23424
MikeMcQuaid wants to merge 1 commit into
mainfrom
fix-rspec-mocks-lifecycle

Conversation

@MikeMcQuaid

Copy link
Copy Markdown
Member
  • Homebrew.default_download_queue memoizes its queue on the Homebrew module, so an example stubbing Homebrew::DownloadQueue.new at first use leaked an RSpec double into later examples and the at_exit shutdown hook, randomly crashing test runs with RSpec::Mocks::OutsideOfExampleError after every example passed.
  • Drop the memoized queue after every example instead; calling shutdown in the around hook's ensure would itself run outside the rspec-mocks lifecycle.
  • Add an ordered regression spec covering the leak.

  • Have you followed our Contributing guidelines?
  • Have you checked for other open Pull Requests for the same change?
  • Have you explained what your changes do? Performance claims (e.g. "this is faster") must include Hyperfine benchmarks.
  • Have you explained why you'd like these changes included, not just what they do?
  • For bug fixes, have you given step-by-step brew commands to reproduce the bug?
  • Have you written new tests (excluding integration tests)? Here's an example.
  • Have you successfully run brew lgtm (style, typechecking and tests) locally?

  • I did not use AI/LLM to create this PR, or I disclosed the tool/model below and reviewed its output; I did not attribute commits to AI and will answer maintainer questions and review comments myself without AI/LLM.

Claude Fable 5 Max with local review and testing.


Copilot AI lite review requested due to automatic review settings August 4, 2026 10:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a test-suite flake caused by Homebrew.default_download_queue memoising a queue object across examples: if a spec stubs DownloadQueue.new on first use, the memoised value can remain an RSpec double and later trigger RSpec::Mocks::OutsideOfExampleError via the at_exit shutdown hook.

Changes:

  • Add Homebrew.reset_default_download_queue to clear the memoised default queue between examples.
  • Reset the memoised default download queue in spec_helper.rb after each example.
  • Add an ordered regression spec to ensure a stubbed queue from one example does not leak into the next.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
Library/Homebrew/test/spec_helper.rb Resets the memoised default download queue after each example to prevent RSpec double leakage.
Library/Homebrew/test/download_queue_spec.rb Adds an ordered regression spec covering the memoisation leak across examples.
Library/Homebrew/download_queue.rb Introduces a reset helper for the memoised default download queue.

馃挕 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread Library/Homebrew/download_queue.rb
Comment thread Library/Homebrew/test/spec_helper.rb Outdated
- `Homebrew.default_download_queue` memoizes its queue on the `Homebrew`
  module, so an example stubbing `Homebrew::DownloadQueue.new` at first
  use leaked an RSpec double into later examples and the `at_exit`
  shutdown hook, randomly crashing test runs with
  `RSpec::Mocks::OutsideOfExampleError` after every example passed.
- Shut down and drop the memoized queue after every example instead.
  A leaked double is only dropped as it cannot receive `shutdown`
  outside the per-example rspec-mocks lifecycle.
- Add ordered regression specs covering the leak and the reset.
@MikeMcQuaid
MikeMcQuaid force-pushed the fix-rspec-mocks-lifecycle branch from c9bd5b5 to 2f8502f Compare August 4, 2026 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants