Bound e2e test parallelism, set the MQTT connect timeout explicitly, and improve failure diagnostics

[Ewerton Scaboro da Silva (ewertons)]

Addresses four items on ADO 39339528. All four are about a failure arriving with nothing that explains it, or a failure that is nobody's fault but the harness's.

1. Bound the e2e test parallelism so the embedded proxies are not starved

This is the one that should fix the long-running red on main.

The e2e tests run in a single JVM with parallel=both and useUnlimitedThreads, so there was no cap on how many ran at once. Several of them stand up an embedded proxyee server inside that same JVM — ConnectionTests on 8899/9000, TokenRenewalTests on 8898, FileUploadTests on 8897, MultiplexingClientTests on 8849. Those proxies must be scheduled promptly to forward traffic, and a burst of test threads starves them of CPU on a two core hosted agent.

That explains the failure signature that has been on main for months. Only the proxied variants ever fail, and they fail by connecting and then having nothing flow — not by failing to connect. It is not specific to one protocol or one class:

Test	Class
CanOpenConnection[MQTT_WS_SAS_DEVICE_CLIENT_true_true]	ConnectionTests
CanOpenConnection[MQTT_WS_SAS_DEVICE_CLIENT_true_false]	ConnectionTests
getAndCompleteSasUriWithoutUpload[HTTPS_SAS_true]	FileUploadTests
getSasUriWithoutUploadConnectionToggle[HTTPS_SAS_true]	FileUploadTests
tokenRenewalWorks	TokenRenewalTests

Three classes, three separate proxies, both HTTPS and MQTT_WS — and in every case the non-proxied variant of the same test passes in the same run. A non-proxied client talks straight to Azure and needs no local CPU to make progress, which is why it is unaffected.

This is expected to cost nothing in run time

I measured rather than guessed. On build 162050 (a successful run):

Total CPU across all tests	49.1 min
Actual wall clock	12.8 min
Average concurrency achieved	3.8x
tokenRenewalWorks alone	12.8 min

useUnlimitedThreads was not buying throughput — the suite only ever reached 3.8x. And the wall clock is floored by tokenRenewalWorks, which takes 12.8 minutes on its own because it deliberately waits out a SAS token expiry.

Predicted wall = max(longest test, totalCPU / threadCount):

threadCount	predicted	change
2	24.6 min	+92%
3	16.4 min	+28%
4	12.8 min	none
6 (chosen)	12.8 min	none
8	12.8 min	none

49 CPU-minutes over 6 threads is 8.2 minutes, comfortably inside the 12.8 minute floor, leaving ~4.6 minutes of scheduling slack. 6 was chosen to sit above the 3.8x average rather than right on it.

This does not make a slow agent fast. It stops an unbounded burst of test threads from starving a server the same JVM depends on.

2. The MQTT connect timeout was whatever Paho happened to default to

MqttIotHubConnection built its MqttConnectOptions without ever calling setConnectionTimeout, so the value was chosen by the library rather than by us, and would change silently underneath us if Paho changed it.

It also needs to stay below the 60s that Mqtt.connect waits for the CONNECT to be acknowledged. If Paho were given as much time as the outer wait, a connection that cannot be established would surface as the outer wait expiring, reporting only Timed out waiting for a response from the server — exactly the unhelpful message the recent tokenRenewalWorks failures came back with.

It is now derived from Mqtt.CONNECTION_TIMEOUT so the two cannot drift. The value works out to 30s, which is what Paho was already applying, so behaviour is unchanged today.

constructorSetsConnectTimeoutShorterThanTheOverallConnectTimeout verifies the call and asserts the relationship holds. Deleting the setConnectionTimeout line fails it with MissingInvocation.

3. CanOpenConnection timing out with no diagnostics

The work item originally described this as a race between the SDK's 60s timeout and the test's @Test(timeout = 60000), fixable by changing one of the numbers. That was my own diagnosis and it was wrong.

These tests call open(true), and the default retry policy is ExponentialBackoffWithJitter with retryCount = Integer.MAX_VALUE. A connection that never succeeds is never abandoned, so the client never throws. Nothing ends the test but the JUnit timeout, and no timeout value changes that — raising it would only make it take longer to say:

org.junit.runners.model.TestTimedOutException: test timed out after 60000 milliseconds
    at ...ConnectionTests.CanOpenConnection(ConnectionTests.java:244)

The reason is only visible through the connection status change callback, so the two tests that open a client now register one and log every transition with reason and cause.

4. Skipped tests all said the same thing

Seven rules decide whether an integration test runs, and every one skipped with "Test is ignored", so a skipped test told you nothing about which rule fired or why. Each now names the annotation or environment variable responsible, following what ErrInjTestRule already did.

I deliberately did not enable @FlakeyTest tests in PR builds. Turning known-flaky tests back on would make PR builds less reliable, which is the opposite of the point. Only four tests are affected, and they still run in non-PR builds.

Verification

• iot-device-client unit tests: 959 + 1 new = 0 failures on JDK 8.
• New unit test passed in CI on the previous push, and fails with MissingInvocation when the fix is removed.
• CanOpenConnection passed 28/28 variants on every JDK in CI on the previous push, confirming the status-callback change is sound.
• mvn -pl iot-e2e-tests/common -am test-compile on JDK 8: BUILD SUCCESS; POM parses with parallel=both, threadCount=6, useUnlimitedThreads removed.

Note on the earlier red run: the three failures on the previous push were a Maven Central TLS failure (Windows), emulator DNS failure (Android), and tokenRenewalWorks — i.e. two infra flakes and the very issue item 1 above addresses.

[Copilot]

Pull request overview

This PR improves MQTT connection diagnostics and integration-test skip reporting.

Changes:

• Explicitly configures and tests the Paho connection timeout.
• Logs connection status transitions in connection tests.
• Replaces generic skip messages with specific explanations.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
MqttIotHubConnectionTest.java	Tests explicit timeout configuration.
MqttIotHubConnection.java	Configures Paho’s connection timeout.
Mqtt.java	Exposes and documents the overall timeout.
ConnectionTests.java	Logs connection status and failure causes.
StandardTierHubOnlyTestRule.java	Clarifies standard-tier skips.
IotHubTestRule.java	Clarifies IoT Hub test skips.
FlakeyTestRule.java	Clarifies flaky-test skips.
DigitalTwinTestRule.java	Adds a skip explanation, but names the wrong setting.
DeviceProvisioningServiceTestRule.java	Clarifies provisioning-test skips.
ContinuousIntegrationTestRule.java	Clarifies CI-test skips.
BasicTierHubOnlyTestRule.java	Clarifies basic-tier skips.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Ewerton Scaboro da Silva (ewertons)]

/azp run Java Linux, Java Windows, Java Android

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).

[Ewerton Scaboro da Silva (ewertons)]

Validation of the parallelism change

All 33 checks are green, including horton-java-gate. The interesting part is the measured effect of threadCount=6, which I can now compare directly rather than predict.

Same PR, same commit content, only the failsafe config differs — build 162059 (useUnlimitedThreads) versus build 162089 (threadCount=6):

JDK	unlimited threads	threadCount=6
11	157 passed, 1 failed, 14.9 min	158 passed, 0 failed, 12.8 min
17	158 passed, 0 failed, 12.8 min	158 passed, 0 failed, 12.6 min
21	158 passed, 0 failed, 12.8 min	158 passed, 0 failed, 12.8 min
8	2273 passed, 0 failed, 0.3 min	2273 passed, 0 failed, 0.3 min

Three things worth pointing out:

No run time cost. This was the open question and the prediction held exactly. Wall clock is unchanged, and JDK 11 is actually 2 minutes faster — because the failure it used to hit was burning ~15 minutes before giving up.

No coverage lost. Passed counts are identical (158 and 2273), and the collected set is 498 distinct test names in both. Worth stating explicitly because a thread cap is exactly the kind of change that could silently drop tests.

The failure it targets went away. JDK 11's single failure under unlimited threads was tokenRenewalWorks, one of the proxied tests this change is about.

I want to be careful not to overclaim on that last point. This failure is intermittent — roughly 50% of nightly main builds over the last 22 pre-1859 runs — so one green run is consistent with the fix but is not on its own proof that it is gone. What the run does establish is that the change costs nothing, which was the reason to hesitate. The nightly main builds will settle whether the flake is actually fixed.

For reference, the earlier red run on this PR (build 162059) was: Maven Central TLS handshake failure on Windows, emulator DNS failure on Android, and this tokenRenewalWorks failure on Linux JDK 11. Two infra flakes and the issue this PR now addresses.

[Ewerton Scaboro da Silva (ewertons)]

Thanks — the review summary flagged DigitalTwinTestRule.java as "Adds a skip explanation, but names the wrong setting", and that was correct. Fixed in 1fe1cfb.

The message said RUN_DIGITAL_TWIN_TESTS. The variable that actually controls it is RUN_DIGITAL_TESTS:

public static boolean runDigitalTwinTests =
    Boolean.parseBoolean(Tools.retrieveEnvironmentVariableValue("RUN_DIGITAL_TESTS", "true"));

The field name and the environment variable name differ, and I wrote the message from the field name without checking. That is exactly the failure mode this change is meant to prevent, so it is worth catching.

Since I clearly could not trust my recollection here, I audited all seven rules against the real mapping in IntegrationTest rather than just patching the one that was reported:

Rule	Message names	Correct?
DeviceProvisioningServiceTestRule	RUN_PROVISIONING_TESTS	yes
DigitalTwinTestRule	RUN_DIGITAL_TESTS	yes, after this fix
ErrInjTestRule	RUN_ERRINJ_TESTS	yes, pre-existing
IotHubTestRule	RUN_IOTHUB_TESTS	yes
BasicTierHubOnlyTestRule	annotation, not a variable	n/a
StandardTierHubOnlyTestRule	annotation, not a variable	n/a
FlakeyTestRule	annotation, not a variable	n/a

The three keyed off isBasicTierHub and isPullRequest deliberately name the annotation rather than a variable, because those are not set from the environment in the same way.

mvn -pl iot-e2e-tests/common -am test-compile on JDK 8: BUILD SUCCESS.