C++ Ice/background test failures with CI about ice HOT 3 CLOSED

What are we testing here?

We call backgroundController->pauseCall("findAdapterById"); to make the locator call block until resume is called.

Then, we test that the call to opAsync doesn't complete immediately, which sometimes fails on Windows CI.

My understanding is that findAdapterById isn't called here because the adapter ID is already in the locator cache.

Using chrono::milliseconds(0) is problematic, as it seems to assume that if the locator didn't block, the call must complete immediately, which is a brittle assumption.

If I add a large delay, the test fails consistently, which shouldn't happen if the test's assumption was correct.

If I disable the locator cache, the test works, even with the large delay.

I don't see how testing locator and router facilities is relevant in this test, I think we better remove them from this test.

from ice.

It's surprising that we get this failure now. Did we ever get it before?

The tests from Ice/background are there to make sure that an asynchronous invocation never blocks the application thread. We tests all the conditions where the async call could be blocking: hanging locator/router, various spots that could hang on connection establishment, etc.

If the locator or router hangs, opAsync should always return without blocking and the returned std::future should never be "ready", at least not until the locator/router call returns. That's what the locator and router tests are about.

My understanding is that findAdapterById isn't called here because the adapter ID is already in the locator cache.

Why would it already be in the locator cache? Because of the ice_ping call which is called before? In theory the Test adapter ID should not be cached because the findAdapterById call should fail with an invocation timeout.

Using chrono::milliseconds(0) is problematic, as it seems to assume that if the locator didn't block, the call must complete immediately, which is a brittle assumption.

wait_for(chrono::milliseconds(0)) is just a way to check the status of the future. The status should never be equal to ready since the adapter ID shouldn't be cached and the opAsync call should always hang on the "under-the-hood" locator findAdapterId call.

It does sound like the adapter ID is indeed cached but it's not clear to me when. If you can easily reproduce, it would be good to figure out if and when the adapter ID gets cached before opAsync is called.

from ice.

This call to ice_ping is causing the cache of the adapterId

The original code in 3.7, is a bit different

The 3.7 version pings the locator proxy, and sets a locator in the locator proxy (there is no need to set the locator there), the code in main is incorrectly sending a ping to "background@Test".

wait_for(chrono::milliseconds(0)) is just a way to check the status of the future.

Right, but I think a large delay 10ms would have catch this issue early.

from ice.