Skip to content

refactor(test): rework Sentry.Test to use Bypass-based HTTP testing#1030

Merged
solnic merged 18 commits intomasterfrom
refa/sentry-test-bypass-rework
Apr 8, 2026
Merged

refactor(test): rework Sentry.Test to use Bypass-based HTTP testing#1030
solnic merged 18 commits intomasterfrom
refa/sentry-test-bypass-rework

Conversation

@solnic
Copy link
Copy Markdown
Collaborator

@solnic solnic commented Apr 2, 2026

Replaces the in-memory event interception approach in Sentry.Test with real HTTP-level testing via Bypass. Events now flow through the full SDK pipeline—encoding, envelope construction, and HTTP transport—before being captured by a local Bypass server and decoded back into structs for assertions.

Motivation

The previous Sentry.Test implementation used NimbleOwnership and an in-process collection mechanism that short-circuited the Client.encode_and_send/4 path. This meant test assertions ran against events that never passed through the actual transport layer, masking potential serialization or envelope-packaging bugs.

Furthermore, there were various issues with config isolation causing test flakiness.

Key changes

Sentry.Test rewrite — The module now spins up a per-test Bypass instance via setup_sentry/1, wires before_send / before_send_log callbacks to capture event structs, and points the DSN at the local endpoint. Collection state is stored in an isolated ETS table per test process.

Sentry.Test.Registry (new) — A GenServer started in the OTP supervision tree when test_mode: true. It manages the ETS table for collectors and boots a global default Bypass instance so that tests without setup_sentry/1 still get a valid DSN (preserving the old {:ok, ""} return value).

Sentry.Client simplified — All Sentry.Test.maybe_collect/1 branches removed from encode_and_send/4. Events always go through the normal transport path; test capture happens at the HTTP boundary instead.

nimble_ownership dependency removed — No longer needed since collection is ETS-based rather than ownership-based. ETS is simpler and avoids the complexity of ownership transfer/allowance bookkeeping across process boundaries; since events are now captured at the HTTP layer by Bypass (which already handles multi-process requests), a shared ETS table keyed by test PID provides natural isolation without extra machinery.

Backward compatibilitystart_collecting_sentry_reports/0, pop_sentry_reports/0, pop_sentry_transactions/0, and pop_sentry_logs/0 continue to work. The owner_pid parameter on these functions is deprecated (no longer meaningful with ETS-based collection). A dedicated test_backward_compat_test.exs validates the legacy workflow.

Follow-up

A separate PR will be opened later on with convenient helpers and improved tests that cover more ground giving us more confidence with the full stack exercised during tests.

It will also make it much nicer to use Sentry test mode in user's test suites 🎉

@solnic solnic force-pushed the refa/sentry-test-bypass-rework branch 3 times, most recently from fa888c5 to aa225a0 Compare April 3, 2026 14:22
@solnic solnic marked this pull request as ready for review April 7, 2026 08:34
cursor[bot]

This comment was marked as resolved.

@solnic solnic force-pushed the refa/sentry-test-bypass-rework branch from aa225a0 to d6c78a8 Compare April 7, 2026 11:08
cursor[bot]

This comment was marked as resolved.

@solnic solnic force-pushed the refa/sentry-test-bypass-rework branch 2 times, most recently from 4db6836 to 44bcb01 Compare April 7, 2026 13:27
@solnic solnic force-pushed the refa/sentry-test-bypass-rework branch from 44bcb01 to b7291dd Compare April 7, 2026 13:49
@solnic solnic force-pushed the refa/sentry-test-bypass-rework branch from 12ea13f to 2d7fdbe Compare April 7, 2026 20:31
cursor[bot]

This comment was marked as resolved.

@solnic solnic force-pushed the refa/sentry-test-bypass-rework branch from c3a29ea to 5ee699d Compare April 8, 2026 07:44
@solnic solnic force-pushed the refa/sentry-test-bypass-rework branch from 5ee699d to 9efc083 Compare April 8, 2026 09:35
cursor[bot]

This comment was marked as resolved.

@solnic solnic force-pushed the refa/sentry-test-bypass-rework branch 2 times, most recently from e0458c4 to 5690133 Compare April 8, 2026 10:48
cursor[bot]

This comment was marked as resolved.

solnic and others added 2 commits April 8, 2026 12:29
Rewrite Sentry.Test to use Bypass for HTTP-level testing instead of
in-memory event collection via NimbleOwnership. Events now travel the
full pipeline (Client -> Transport -> HTTP) and are captured at the
Bypass endpoint, while before_send callbacks capture full Elixir structs
for assertion convenience.

Key changes:
- Rewrite Sentry.Test with Bypass-backed setup_sentry/1 as primary API
- Remove maybe_collect/1 calls from Client and Scheduler
- Remove test_mode? bypass from Sentry.capture_exception/capture_message
- Deprecate test_mode config option
- Remove nimble_ownership dependency
- Convert all test files to use Bypass-based assertions
- Add Bypass envelope collector helpers to test support modules
- Update integration tests to use HTTP envelope assertions

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@solnic solnic force-pushed the refa/sentry-test-bypass-rework branch from 587c617 to ceb4ba4 Compare April 8, 2026 12:29
@solnic solnic force-pushed the refa/sentry-test-bypass-rework branch from e88434f to c7b049a Compare April 8, 2026 14:37
try do
backend.handle_event(log_event, config, handler_id)
rescue
_ -> :ok
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

so this will swallow internal problems completely silently?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@sl0thentr0py so, sometimes logger handler would crash when running tests, and I thought we don't want that given the general contract of making sure our tooling doesn't crash people's code, but this is for sure out of scope here, so I reverted it, and taking a note instead.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we should not crash people's code indeed, but if there's an underlying problem with the handler we need to fix that first.

@solnic solnic merged commit cd5c019 into master Apr 8, 2026
11 checks passed
@solnic solnic deleted the refa/sentry-test-bypass-rework branch April 8, 2026 15:21
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