Update HttpUrlConnection androidTests to make them compatible for parallel running #500
Conversation
surbhiia
changed the title
surbhiia
changed the title
There was a problem hiding this comment.
I'm struggling with this. I think it's a pretty strong antipattern to mix test code explicitly into the production code, which includes HttpUrlConnectionSingletons. I know we sometimes expand visibility to facilitate testing, but it's not that common and this seems like a considerably deeper intrusion.
I'd like to give some additional consideration to how me might solve this concurrency challenge without compromising the instrumentation code.
| if (testOpenTelemetry != null) { | ||
| return testOpenTelemetry; | ||
| } else { | ||
| return openTelemetryInstance; | ||
| } |
There was a problem hiding this comment.
nit: always prefer uninverting the if and omit the redundant else.
It is an interesting problem! I will also look into other ways of solving it in a cleaner way. :) |
|
Closing this PR as per our discussion in Android Sig today morning. One possible approach we discussed we could look at for future is - Using roboelectric to test most of the functionality without BCI (as BCI might not be possible with it) and having 1 androidTest to test that BCI works (end-to-end integration). For now, we will have 6 androidTests for httpurlconnection that can be run synchronously. |
Improving androidTests to make them compatible for running in parallel:
Also tested the actual instrumentation (non test scenario) works as expected.
No change for the Okhttp3 androidTests. They run correctly synchronously: