Oban telemetry start event is not needed + tests

[mrdotb]

👋

The oban job metadata can be accessed from the exception event metadata so it's not needed to attach to the start event.

Testing the oban integration is not easy since Application.spec is used directly so I wrapped it inside a Utils module than I can mock in order to simulate a oban event.

[odarriba]

Thanks for your time on this change! 🫶

I will mark this one as changes required because we cannot remove the start event as users may lose context that they have now on their manually reported errors.

Aside from that, I think we can avoid the mock and I'm not sure if the testing strategy is correct or not (however, taking the time to introduce tests is really appreciated 🤗 )

[odarriba]

I'm not sure if this is the proper way to test this. Integration tests are missing because we do not have yet a clear idea of how to test it.

ideally, we would need to spin up an Oban job using Oban itself, and ensure the library is the one that emits events.

If we do that instead, any change in Oban will break the integration and the tests will continue to pass.

Let me ping @crbelaus to see his thoughts on this.

[mrdotb]

Mocking the :telemetry is the way chosen on https://github.com/appsignal/appsignal-elixir/blob/main/test/appsignal/oban_test.exs so it's a good reference

[crbelaus]

Thanks for dedicating your time to write this pull request, but I do not agree with this approach. I feel like this test too complicated because it is mixing multiple different concerns.

On the one hand it is testing the Telemetry package itself by checking first if an event handler is attached and then if sending an event effectively sends it to the attached handler.
Telemetry already tests this. If I call :telemetry.attach it is safe to assume that the event handler is attached, otherwise there would be a bug in the Telemetry package. Same happens with :telemetry.execute.

On the other hand we are not testing if the ErrorTracker captures the error, just that it emitted a particular Telemetry event. Something that is already tested elsewhere.

---

The way I see it, I would directly call ErrorTracker.Integrations.Oban.handle_event/4 with the desired information and then check if the expected behaviour happens (either the context is updated or an error is recorded).