Shopify / statsd-instrument

A StatsD client for Ruby apps. Provides metaprogramming methods to inject StatsD instrumentation into your code.
http://shopify.github.io/statsd-instrument
MIT License
570 stars 94 forks source link

Refactor assert_statsd_calls to flunk if an exception occurs inside the block #193

Closed wvanbergen closed 4 years ago

wvanbergen commented 4 years ago

Ref #166 & #184.

This is a more straightforward way of dealing with exception that our previous approach. We tell people they should nest assert_raises inside the statsD assertion block.

This is a backwards incompatible change though - it may cause some tests to fail that previously were succeeding. However, the fix for those is easy: change the nesting order. The assertion message will tell you how to fix it.

# - This used to succeed in version < 2.3.3
# - This fails due to the lack of statsd increment in version >= 2.3.3
# - This will fail due to an unhandled exception after this PR is merged
assert_raises(RuntimeError) do
  assert_statsd_increment('foo') do
    raise 'error'
  end
end

# - This used to succeed in version < 2.3.3
# - This also succeeds in version >= 2.3.3
# - This will fail due to an unhandled exception after this PR is merged
assert_raises(RuntimeError) do
  assert_statsd_increment('foo') do
    StatsD.increment('foo')
    raise 'error'
  end
end

# - This behaves the same (succeeds) in all versions
assert_statsd_increment('foo') do
  assert_raises(RuntimeError) do
    StatsD.increment('foo')
    raise 'error'
  end
end

# - This behaves the same (fails) in all versions
assert_statsd_increment('foo') do
  assert_raises(RuntimeError) do
    raise 'error'
  end
end
wvanbergen commented 4 years ago

Pushed a change to simplify the tests by getting rid of the helpers. Docs should be improved but I feel that's a bit outside the scope of this PR.