[TEST] Fix ext.http.curl.BasicCurlHttpTests.ElegantQuitQuick flaky test

[dbarker]

The following test is timing based and flaky in CI. This test should be fixed or disabled.

 898/1247 Test  #898: ext.http.curl.BasicCurlHttpTests.ElegantQuitQuick .........................................................................***Failed    0.05 sec
Running main() from /private/tmp/googletest-20250808-4877-y4cffa/googletest-1.17.0/googletest/src/gtest_main.cc
Note: Google Test filter = BasicCurlHttpTests.ElegantQuitQuick
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from BasicCurlHttpTests
[ RUN      ] BasicCurlHttpTests.ElegantQuitQuick
/Users/runner/work/opentelemetry-cpp/opentelemetry-cpp/ext/test/http/curl_http_test.cc:681: Failure
Value of: cost < std::chrono::milliseconds{20}
  Actual: false
Expected: true
cost ms: 21 libcurl version: 0x80600

[  FAILED  ] BasicCurlHttpTests.ElegantQuitQuick (43 ms)
[----------] 1 test from BasicCurlHttpTests (43 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (43 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] BasicCurlHttpTests.ElegantQuitQuick

 1 FAILED TEST

[owent]

Could we increase the timeout here? GitHub CI runners sometimes schedule CPUs slowly, and a slightly higher timeout might help prevent flaky failures.

[dbarker]

Could we increase the timeout here? GitHub CI runners sometimes schedule CPUs slowly, and a slightly higher timeout might help prevent flaky failures.

We could but it may be helpful to check and see if the test can be made more deterministic. I'd like to reduce the timeouts and sleeps in unit tests :)

[github-actions]

This issue is available for anyone to work on. Make sure to reference this issue in your pull request.
✨ Thank you for your contribution! ✨

[yogarajalakshmi-s]

Hi @dbarker! Can you assign this to me?

And agreed! A deterministic fix beats raising the timeout and that would likely just mask the flakiness on slower runners. I'll look at replacing the 20ms wall-clock assertion in ElegantQuitQuick with something that isn't timing-dependent, like mocking the clock. Is there an existing time-mocking pattern in the test suite I should follow?

[thc1006]

I filed #4342 for this without searching first, which was my mistake. Closing that one. Two measurements from it are worth having here, and there is an answer to the tooling question above.

The overshoot is not marginal. The sighting I hit measured cost ms: 247 against the 20 ms bound, where the report in this issue measured 21 ms. At 247 ms there is no bound that both survives a loaded runner and still means anything, so raising it does not close this. That is evidence for @dbarker's preference over the simpler option.

It is runner scheduling, not build configuration. CMake gcc 14 (maintainer mode, async) in run 30773626013 invokes ctest twice against the same build directory with no rebuild in between. The first pass reported 100% tests passed, 0 tests failed out of 1348 and the second reported this test failing at 247 ms. Same binaries, opposite results.

@yogarajalakshmi-s on the time mocking question: there is nothing to follow. Grepping sdk, api, ext, exporters and functional for a mock, fake or injected clock, a clock provider, or a mocked Now() returns nothing, so the tree has no clock abstraction today and adding one for this test would be its first.

One observation that may help without a clock. The 20 ms is standing in for something the test states in its own comment: the wait "should be less than scheduled_delay_milliseconds_", so what it means to check is that FinishAllSessions() woke the background thread rather than letting it sleep out its poll interval. That is a property of the wakeup rather than of the elapsed time, and asserting it is what would make the case immune to a loaded runner.

This is yours, I am not picking it up. Just leaving the measurements where they are useful.

[dbarker]

This test is still failing periodically in CI. The next step will be to disable it if no fix is available soon.