Fix flaky test: no telemetry by default in test suite

[richardm-stripe]

Our CI in github actions has ~consistently been failing tests like this. This hasn't been consistent across time, and it doesn't happen in Travis CI (which we are trying to deprecate).

This happens in RequestTelemetry.java, which in my opinion shouldn't be enabled when we are running tests.

This disables telemetry throughout tests, which causes CI to pass.

I do want to better understand the cause of why the NullPointerException is triggering in this environment but not locally, but have not yet identified it.

[remi-stripe]

fine with the change but that means we could break telemetry and not notice right? Or do we have enough coverage in tests specifically for this?

[richardm-stripe]

Good point, we have some tests that explicitly test enabled/disabled Telemetry. Still, I don't really want to merge this until I actually understand the error.

[richardm-stripe]

Ok, I now understand what is going on.

In RequestTelemetry.java there is a piece of global state:

private static ConcurrentLinkedQueue<RequestMetrics> prevRequestMetrics

which can affect the behavior of RequestMetrics.getHeaderValue. Sometimes it returns empty, sometimes not, depending on previous/concurrent requests that have been made throughout the test suite. (This is why I can never reproduce the failure running the test in isolation).

When it's not empty, then requestWithTelemetry adds a header to the request argument before passing it to this.request.

This prevents the Mockito matcher from applying, and so the mocked client just returns null instead of providing a response. Attempting to call a method on the null response triggers the null pointer exception.

---

Given the role of Mockito, this does not represent an issue that could arise production. Disabling Telemetry by default for requests is a reasonable solution, because that removes the global state/non-determinism here.