Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to override/mock the Client #1810

Open
erawhctim opened this issue Feb 20, 2023 · 7 comments
Open

Add ability to override/mock the Client #1810

erawhctim opened this issue Feb 20, 2023 · 7 comments
Labels
backlog We hope to fix this feature/bug in the future feature request Request for a new feature

Comments

@erawhctim
Copy link

erawhctim commented Feb 20, 2023

Description

It's difficult to write unit tests for Bugsnag functions due to how the SDK is built and how the Client is initialized/managed.

#103 describes a solution - building a wrapper class around the Bugsnag SDK and mocking that in tests - which is a good strategy that works well for most testing use cases. However, when we want to actually unit test our Bugsnag SDK wrapper class, we need a way to mock Bugsnag's internals, which is difficult to do today (especially in a plain JVM unit test without access to Android APIs/Context/etc.

Describe the solution you'd like

The ability to set an arbitrary Client implementation within Bugsnag, so that Bugsnag.addOnError and/or Bugsnag.notify can be called without the SDK complaining about start not being called (or requiring an API key).
If Client was an interface that we could override, I think that would be a great start.

Additionally, it would be even better if there was an optional bugsnag-test artifact that provided a MockClient implementation to use and assert upon in tests.

Describe alternatives you've considered

  • Static mocks on Context and Bugsnag, essentially mimicking the behavior of the SDK and setting no-op's or reasonable defaults. This works, but it's tedious.
  • The recommendation in this comment (always set up the SDK with the API key, and just use release stages to gate the error upload functionality)
    • This is closest to the approach we landed on

Additional context

N/A

@johnkiely1
Copy link
Member

Hi @erawhctim,

We are giving this some consideration and we have added an item to the backlog to add some additional in depth information to our docs around our recommended ways on how to mock BugSnag.

Appreciate it may be a little tedious in places but for now our recommended suggestion would be to use static mocks and utility methods, like you suggested.

We will post here with updates.

@johnkiely1 johnkiely1 added feature request Request for a new feature backlog We hope to fix this feature/bug in the future labels Mar 3, 2023
@erawhctim
Copy link
Author

Ok, that makes sense. Thanks for the update!

@erawhctim
Copy link
Author

Has any internal discussion moved forward on this? Or is mocking still the preferred method of replacing BugSnag SDK functionality in tests?

@lemnik
Copy link
Contributor

lemnik commented Apr 5, 2024

Hi @erawhctim

Thanks for bumping this. We still don't have anything scheduled, but it would good to get a clear idea of what will help.

We'd ideally like to keep any "test specific" logic out of bugsnag-android-core if possible, I'd like to know if a way to side-channel this functionality would cover what you need. Something along the lines of a new bugsnag-android-testing module that would allow:

BugsnagTesting.setGlobalClient(myMockedClient)
BugsnagTesting.clearGlobalClient()

Would that be enough? Or would you need some other functionality?

@erawhctim
Copy link
Author

That would be great! I agree that offering a separate testing client makes the most sense 💯

Ideal use case (IMO):

  • Client would be an interface that we could implement ourselves, avoiding the need to use mocking frameworks.
  • The testing artifact would allow me to clear the Client and set it to null (to help simulate the "you must call Bugsnag.start" error scenarios)

Right now there isn't any other functionality we would need; all the Bugsnag.something calls end up routed through to the underlying Client right?

@lemnik
Copy link
Contributor

lemnik commented Apr 5, 2024

Unfortunately we're not likely to make Client an interface for the foreseeable future, as that would be a major breaking change in the API (as nice as it might be from an extensibility perspective).

Other than Bugsnag.start, Bugsnag.isStarted, and Bugsnag.getClient all methods on Bugsnag are just proxies to the global Client. It's entirely possible to skip using the Bugsnag class entirely, and use a Client instance instead (its constructors follow the same pattern as Bugsnag.start)

@erawhctim
Copy link
Author

erawhctim commented Apr 5, 2024

got it, ok. Yeah if we still need to rely on Mocking tools to control a test-specific instance of Client, I'm not sure if offering a testing artifact would change anything over what we do today (unfortunately).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog We hope to fix this feature/bug in the future feature request Request for a new feature
Projects
None yet
Development

No branches or pull requests

3 participants