Azure DevOps Test Plan sync: a failed publish is indistinguishable from a successful one

[Wladefant]

Summary

When UpdateResultsToTM is set to AzureDevOps TestPlan, there is no path by which a publishing failure can reach the user, the report or the exit code. An authentication failure, an unresolvable test point, a rejected request or a network error all produce exactly the same observable outcome as a fully successful publish: a green run and a console line that says the upload was starting.

That matters because the reason to publish results into a test plan is usually to have them serve as the record of what was tested. A record that may or may not exist, with nothing distinguishing the two cases, cannot serve that purpose.

Root cause — four independent layers, each of which alone is sufficient

1. The only success signal is returned before any network call happens.

AzureSync.updateResults buffers into a list and returns true unconditionally:

@Override
public boolean updateResults(TestInfo tc, String status, List<File> attach) {
    AzureTestData test = new AzureTestData(project, testPlanId, tc.testScenario, tc.testCase, status, attach);
    listOTest.add(test);
    return true;
}

That return value is the only thing the reporting layer checks — SummaryReport.updateTMResults:196 writes "Unable to Update Results" into the test log when it is false, which it never is. The real HTTP work happens later, in disConnect(), long after that check.

2. Every method that does the HTTP work swallows its exceptions and returns normally.

In AzureClient, all of createNewTestRun, updateResults, sendAttachment, updateRunStatus, getResultId, getTestSuiteId and getTestPointId wrap their whole body in:

} catch (Exception ex) {
    LOGGER.log(Level.SEVERE, null, ex);
}

createNewTestRun returns void, so even a failure to create the test run at all — the very first call — is invisible to its caller. disConnect() does not look at anything and immediately nulls the connection.

3. HTTP status codes are never inspected.

AbstractHttpClient.parseResponse takes the HttpResponse, reads the entity and parses it. It never touches response.getStatusLine(). A 400/401/403/404 whose body happens to be JSON — which is what Azure DevOps returns for most errors — is therefore handed back as if it were a normal result. When the body is not parseable it logs and returns null, and the resulting NullPointerException upstream is caught by layer 2.

4. Nothing downstream examines the outcome.

Control.endExecution calls ReportManager.sync.disConnect() and moves on. The process exit code is unaffected, the HTML report says nothing, and the only trace of the failure is a SEVERE stack trace in the engine log — which nobody reads when the run reports success.

The console output actively suggests success

createNewTestRun prints a progress line per test case:

Azure DevOps: updating //MyScenario/MyTestCase result(Passed) with 4 attachments...

and the line that would have completed it is commented out (AzureClient.java:247):

// System.out.println("Done!");

So the user sees "updating…" for every test case whether or not anything was written, and never sees a completion or a failure. If getResultId returned -1, the same line still prints and the subsequent PATCH still goes out with "id": -1.

Reproducing

No exotic setup is needed — invalidate the credential and run a test set with UpdateResultsToTM=AzureDevOps TestPlan. The run completes, exits 0, prints the "updating…" lines, and nothing in the report or the exit status differs from a run that published correctly. The same holds for an unreachable server, a wrong test plan id, and a test case whose suite or title cannot be resolved (see the companion issue #316, where unresolved lookups feed a literal -1 into the request).

Suggested fix

In rough order of value per line changed:

1. Check the status code. Have parseResponse (or, if that is too broad a blast radius, the Azure client's own call sites) treat a non-2xx response as a failure and surface the status and response body. Almost every real failure becomes diagnosable from this alone.
2. Give createNewTestRun a return value — or let it throw — and have AzureSync.disConnect() propagate it. Track per-test-case outcomes so a partial failure is distinguishable from a total one.
3. Print a summary at the end of the run, e.g. Test management: 42 of 45 results published; 3 failed (unresolved test point: …). This is the smallest change that makes the current silent behaviour visible, and it is worth doing even if the rest is not.
4. Reflect it in the run outcome. Publishing was explicitly requested; failing to do it should be more than a log line. A non-zero exit code would be the strongest signal — if that is too disruptive as a default, a flag to opt into it, or at minimum a clearly marked error in the HTML report.

AzureSync.updateResults honestly can only answer "queued" given the current design, which is fine — but then the check at SummaryReport:196 is dead code and should either be removed or be backed by a real signal.

Minor, in the same code path

disConnect() calls createNewTestRun, which does listOTest.get(0) after the stream. When the list is empty — a run where nothing was published — that throws IndexOutOfBoundsException, which is caught by the same blanket handler and logged as SEVERE with no context. An early return on an empty list would avoid a confusing stack trace in the log.

Environment: release/3.1.0 at 15274331, JDK 17.