Allow warnings to be clear only within a block

[splattael]

Problem

It's common practice to clear the internal state of Warning via Warning.clear.

At GitLab, we use Warning to log deprecation warnings into a separate file.

When testing this very behavior we are resetting the state via Warning.clear.

Since we are planning to use Warning in specs to prevent already fixed deprecation warnings to sneak in again, Warning.clear would clear also these definitions 😞

Example

# frozen_string_literal: true

require 'spec_helper'
require 'warning'

Warning.process('', /foo/ => :raise)

RSpec.describe "test" do
  around do |example|
    example.run
    Warning.clear
  end

  it 'warns about bar' do
    Warning.process('', /bar/ => :raise)
    expect { warn("bar") }.to raise_error(RuntimeError, /bar/)
  end

  it 'warns about foo' do
    expect { warn("foo") }.to raise_error(RuntimeError, /foo/)
  end
end

When running rspec --order defined warning_spec.rb I see a spec failure like:

test
  warns about bar
foo
  warns about foo (FAILED - 1)

Failures:

  1) test warns about foo
     Failure/Error: expect { warn("foo") }.to raise_error(RuntimeError, /foo/)
       expected RuntimeError with message matching /foo/ but nothing was raised
     # ./spec/models/warning_spec.rb:22:in `block (2 levels) in <top (required)>'
     # ./spec/models/warning_spec.rb:11:in `block (2 levels) in <top (required)>'

Finished in 0.0154 seconds (files took 1.21 seconds to load)
2 examples, 1 failure

Proposed solution

Allow Warning.clear to accept a block. In this case the values are cleared only within the block and restored after the block end.

In the example above we could pass a block to Warning.clear { ... } instead:

# frozen_string_literal: true

require 'fast_spec_helper'
require 'warning'

Warning.process('', /foo/ => :raise)

RSpec.describe "test" do
  around do |example|
    Warning.clear { example.run }
  end

  it 'warns about bar' do
    Warning.process('', /bar/ => :raise)
    expect { warn("bar") }.to raise_error(RuntimeError, /bar/)
  end

  it 'warns about foo' do
    expect { warn("foo") }.to raise_error(RuntimeError, /foo/)
  end
end

Happy to open a PR if this solution is accepted ❤️

[splattael]

Note that the code of the proposed solution above does not work with Warning.freeze.

[jeremyevans]

I guess I'm OK with this feature. I don't like the name with_state. Maybe restore_state or temporary_state, which I think are better but still not good (open to other suggestions). An alternative would be having clear take a block, and if a block is passed, it clears changes during the block.

In terms of implementation, you need to dup @ignore, @process, and @dedup (if not false) before yielding. Also, this should be a public method in Warning::Processor (which Warning extends).

[splattael]

@jeremyevans I love the idea of passing a block to Warning.clear so the internal state is cleared only within the block and gets restored afterwards.

I've adjusted the issue title and description.