Should the SDK provide test helpers?

[st0012]

Since error reporting is usually enabled in staging/production environments, users may not write tests for it. And to be honest, there isn't a clear way to test it either (probably only verifying the arguments are sent to Sentry.capture_exception correctly?).

As both a user and a maintainer of the SDK, I sometimes hope there are helpers to verify event payloads when developing apps. Ideally, it'll be something like:

# in addition to inject test helpers, it'll also
# - enable the SDK in a test-only mode (haven't decided what that means yet)
# - replace HTTPTransport with DummyTransport
include Sentry::TestHelper

it "reports the exception to Sentry" do
  expect do
    do_something
  end.to have_sent_event_to_sentry(transaction: "PostsController#create", exception: "ZeroDivisionError")
  
  # or for a more detailed verification
  # captured_sentry_events is defined in Sentry::TestHelper
  last_event = captured_sentry_events.last

  expect(last_event.contexts).to match(....)
end

[st0012]

BTW, to use the helpers, users may need to move sentry-ruby out of the production group and make it universal in all environments (test & production group sounds rare). I wonder if that'll cause concern or even issues for some users.

[benoittgt]

I am in favor of this proposal.

I didn't implement this. But we use this in our apps:

# ~/code~/app/spec/support/sentry.rb
# frozen_string_literal: true

module SentryHelpers

  def sentry_events
    sentry_transport.events
  end

  def last_sentry_event
    sentry_events.last
  end

  def reset_sentry_events
    sentry_transport.events = []
  end

  private

    def sentry_transport
      Sentry.get_current_hub.current_client.transport
    end

end

RSpec.configure do |config|
  config.include SentryHelpers

  config.around(:each) do |example|
    expect(sentry_events.size).to eq 0

    example.run

    reset_sentry_events
  end
end

# spec/support/matchers/sentry_matchers.rb
# frozen_string_literal: true

RSpec::Matchers.define :raise_sentry_event do |expected_exception_attributes|
  supports_block_expectations

  match(notify_expectation_failures: true) do |actual|
    actual.call
    @expected_exception_attributes = expected_exception_attributes

    if (expected_exception_attributes.keys - authorized_attributes_keys).present?
      @unauthorized_keys_error = true
      return false
    end

    if sentry_events.size != 1
      @sentry_events_size_error = true
      return false
    end

    if expect_sentry_exception?
      sentry_exception = last_sentry_event.exception
      if sentry_exception.nil?
        @sentry_events_no_exception_error = true
        return false
      end

      sentry_single_exception = sentry_exception.instance_variable_get(:@values).first

      expect(sentry_single_exception.type).to eq exception_type if exception_type
      expect(sentry_single_exception.value).to eq exception_value if exception_value
    end

    expect(last_sentry_event.extra).to match(extra) if extra
    expect(last_sentry_event.tags).to match(tags) if tags
    expect(last_sentry_event.message).to eq(message) if message
    expect(last_sentry_event.level).to eq(level) if level

    return true
  end

  match_when_negated do |actual|
    actual.call

    expect(sentry_events.size).to be_zero
  end

  failure_message do |_actual|
    if @unauthorized_keys_error
      "Authorized attributes are #{authorized_attributes_keys}. "\
        "Attributes #{expected_exception_attributes.keys - authorized_attributes_keys} are not valid or cannot be tested."
    elsif @sentry_events_size_error
      if sentry_events.size.zero?
        'No sentry events have been raised.'
      else
        "#{sentry_events.size} sentry events have been raised, not just 1."
      end
    elsif @sentry_events_no_exception_error
      'One sentry event has been raised, but it is not an exception, probably a message.'
    end
  end

  failure_message_when_negated do |_actual|
    'Sentry events have been raised but should not have.'
  end

  def expect_sentry_exception?
    exception_type || exception_value
  end

  def exception_type
    @expected_exception_attributes[:exception_type]
  end

  def exception_value
    @expected_exception_attributes[:exception_value]
  end

  def message
    @expected_exception_attributes[:message]
  end

  def extra
    @expected_exception_attributes[:extra]
  end

  def level
    @expected_exception_attributes[:level]
  end

  def tags
    @expected_exception_attributes[:tags]
  end

  def authorized_attributes_keys
    [:exception_type, :exception_value, :message, :extra, :level, :tags]
  end
end

[st0012]

@benoittgt Thanks for the feedback! I guess there's a real need for it 😂

[github-actions]

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!

---

"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

[Flixt]

Having test helpers would definitely greatly improve the barrier (at least I currently have) to write tests for errors being reported, so I love the general idea.

I just wonder if this is something that should also be provided one abstraction layer higher. I also wrote a comment about that into rails/rails#43625 .

[st0012]

@Flixt @benoittgt I've added #1773 to support the most basic test helpers and I'd love to hear your feedback on the API design 🙂

Update: Do you think adding sentry_error_event and sentry_transaction_events will be helpful too?