-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
The API module probably MUST NOT depend on the oteltest
which implements the API
#2077
Comments
I'm not sure I follow. How can we test the API without a mock implementation? |
You have the noop for testing the API, and also tests in the SDK for testing the API implementation |
Allowing the noop implementation instead of the specific testing implementation we include in This package is used in the
Each use would not be supported by the noop implementation as it does not provide the necessary testing framework. Meaning the existing testing utilities provided by the That said, it might be possible to refactor the I'm not convinced this second approach is going to be worth it. I'm guessing it will add a lot of complication and indirection that are ultimately are not necessary only to remove this dependency. But it might be worth exploring to validate this assumption. |
Everything you say here I feel is not as important as the fact that right now I have 2 implementations of the API, and when unittesting my application (integration) I am testing it with a mock that is 90% copy paste from the SDK (but still duplicate code that has bugs, see the #2075), and my code can work great with that but not with the official SDK. I think it is more important to put the user first, and ensure that their tests/unit-tests are testing the right thing and give them the confidence they need, instead of our tests. If we have to duplicate code for our tests that is much better than having this duplicate code be used by all integrations to do their tests. |
Closing as #2111 completed the dependency on the |
No description provided.
The text was updated successfully, but these errors were encountered: