fix(plugin-testops): correlate CI uploads with TestOps job runs end-to-end - #906
Merged
Conversation
This was referenced Sep 1, 2026
todti
force-pushed
the
feat/testops-attach-existing-job-run
branch
2 times, most recently
from
September 1, 2026 15:53
945cb3e to
f39f2a2
Compare
Allure Report Summary
|
Allure perf metricsGenerated at:
Artifacts: allure-perf-metrics |
formaceft-93
approved these changes
Sep 2, 2026
formaceft-93
approved these changes
Sep 2, 2026
…o-end Results uploaded from CI never connected back to the job run TestOps itself tracks, breaking rerun, test-plan-scoped runs, and two-way pipeline triggering. Uploads from Azure DevOps also produced a job with a corrupted name and no working integration, making it impossible to trigger the pipeline back from TestOps. CI descriptor and job binding: - Send ci.type + ci.endpoint (derived from the job URL's origin) so TestOps can attach the right build-server integration to the job; previously neither was sent, so triggering a job from TestOps always failed with "build server is not configured for job". - Send job.name, job.url, jobRun.name, jobRun.url, and the job's numeric id when already known, instead of leaving the job named after its raw uid. - Send Branch (and, on Bitbucket, CustomName) as job/job-run parameters, and restate the job's already-configured parameters on every upload — job.parameters is a full replacement server-side, so omitting them would silently wipe out manual configuration. - Support ALLURE_JOB_UID/URL/NAME and ALLURE_JOB_RUN_UID/URL/NAME/BRANCH env vars to override individual CI-detected fields when a detector gets it wrong or the CI is exotic, and ALLURE_CI_ENV to carry ALLURE_* context across pipeline steps/processes that don't share environment. Job run attachment: - Read ALLURE_JOB_RUN_ID: when TestOps itself started the pipeline (e.g. its "Run" button), attach to that job run instead of creating a disconnected launch, and bind the upload session to it (test_result.job_run_id is what rerun and test-plan matching key off). Also read jobRunId from /api/upload/start's response unconditionally, since the server always resolves one. - Propagate jobRunId onto launch-scoped attachment/error uploads so they're attributed to the run that produced them, not just the launch. - Add a reopenClosedLaunch option to recover from uploading into an already-closed launch instead of failing outright. - Add a launchId option to attach to an existing launch by id. - Skip job/job-run creation entirely for local (non-CI) runs instead of synthesizing a fake job; tag the launch itself as an allure3 report in that case so TestOps has some signal about its origin. Reliability: - Classify upload errors (transient/recoverable/terminal) and retry transient ones with backoff across all upload call sites, replacing a single untargeted retry path. - Add a leaky-bucket rate pacer (requests/files/bytes per window) so large uploads don't overrun TestOps's own limits. - Defer upload batches that exhaust their retry budget to a finalization-time queue instead of dropping them, so a sustained TestOps outage doesn't lose results outright. - Retry the launch-creation prerequisites (/api/upload/start, the job-parameters lookup, the closed-launch check) that previously had no retry at all, unlike every per-chunk upload call. - Validate that TestOps actually returns a launch id when attaching to a job run, and only treat a 404 (not any failure) as "job doesn't exist yet" when looking up existing job parameters, so a transient error can't silently wipe real configuration. Test plan: - New `allure testops-plan` CLI command: when ALLURE_JOB_RUN_ID is set, fetches the job run's selected test cases from TestOps and writes testplan.json in the format allure-js-commons already reads via ALLURE_TESTPLAN_PATH. No-ops when the variable isn't set, and continues without a plan (rather than hard-failing the pipeline) if the fetch itself fails.
…t success run() never removed a name's pending entry when a subsequent direct call for that same name succeeded outright (not suspended, not deferred) - only ever replaced it on another failure. The stale task closure (captured from the earlier failed attempt) would still sit in the queue and get re-run by flush() at finalization, re-uploading data a later successful call already sent. Added a test that catches this by asserting pendingNames is empty after the later success.
… plugin.ts No behavior change - moved code that was already free functions (not methods needing class state) into their own modules, since client.ts and plugin.ts had grown to ~900 lines each: - utils/httpRetry.ts: retryRequest and its error-classification helpers - utils/jobParameters.ts: externalParameters, ciEndpoint, TESTOPS_CI_TYPE - utils/launchCategories.ts: the category enrichment/sync helpers, converted from private plugin.ts methods to functions taking the client/store explicitly instead of implicit `this` TestOpsClient and TestOpsPlugin themselves stay as single classes - splitting their actual methods across files would need composition/ mixins, a real architecture change, not just a move.
…aunch TestOps' LaunchCreateDto has no such fields and Jackson's default fail-on-unknown-properties would reject the request, breaking launch creation for every local (non-CI) run.
…in plugin.ts The onRetry/onDeferred wiring and the isTestOpsClientError/Error/else logging branch were copy-pasted identically at all 4 upload call sites, differing only by a label string.
Three defects found by comparing what the plugin uploads against what the TestOps API accepts. They are independent of each other. Global errors come from the results files test framework adapters write, and their message is optional in our model - nothing validates it on the way in. TestOps requires a non-blank message and rejects the entire bulk request when one item lacks it, so a single untitled error dropped every valid error uploaded alongside it. Send a placeholder instead, keeping the trace. TestOps accepts a launch name of at most 255 characters and fails the request otherwise, so a longer report title killed the upload before it started. Trim the name to the accepted length. The upload API has no `environment` property on a test result, so the value was dropped on arrival; the binding it was meant to carry travels in `namedEnv`, which is sent next to it. Remove the dead field from the payload and from the type that mirrors the API.
…ected CI startUpload checked isLocalCiDescriptor(ci) before jobRunId, so a job run TestOps started itself (its "Run" button) on an agent packages/ci doesn't recognize as any known CI provider took the local-run shortcut instead of attaching to the job run - #launch was never set, and the next createSession() call crashed with "Launch isn't created!". Also fixes externalParameters() sending a job's stale saved Branch/CustomName back to TestOps: the loop that restates existing job parameters ran after the fresh values were set, overwriting them. Drops the inner retry wrapper around the test-result chunk POST, since the outer withUploadRetry in uploadTestResults already retries the whole operation - the two layers disagreed on what's retryable and could redo a successful createNamedEnvs call for nothing. Moves applyAllureCiEnv from plugin-testops into @allurereport/ci so the new testops-plan CLI command can decode ALLURE_CI_ENV too - it previously read ALLURE_JOB_RUN_ID directly, which meant it saw nothing in the matrix-job/ separate-container scenario ALLURE_CI_ENV exists to handle.
The three-way rateLimit resolution (unset -> default, false -> disabled, object -> itself) was one dense ternary; split into a named helper with each case spelled out.
epszaw
force-pushed
the
feat/testops-attach-existing-job-run
branch
from
September 2, 2026 13:53
3491add to
f8e8c6d
Compare
…ps omits it from the response startUpload only bound the session to a job run when /api/upload/start's response echoed jobRunId back, but that field is optional on the response type. If it's missing, createSession silently falls back to a manual session, losing the job-run binding even though we told TestOps which job run to attach to.
…uploads done() logged an error when some upload streams never made it through finalization but continued straight to stopUpload() and, if autoclose was on, polled and closed the launch anyway - reporting an incomplete launch as finished. Skip the close (leave the launch open) when anything is still pending; stopUpload's own CI-status report is unaffected.
…etter locals from the branch's own code
…_JOB_RUN_ID on an undetected CI isManuallyEnabled only checked ALLURE_TESTOPS_ENABLED/CI, so a job run TestOps started itself on an environment with no recognized CI still hit the local-CI disable guard in the constructor and never got a chance to attach - the client-side fix for this exact scenario was unreachable.
On a reused workspace (a self-hosted runner or cached container that doesn't wipe its working directory between runs), a testplan.json left over from an earlier successful run would otherwise still be there for the current run to pick up and execute, even though this run's fetch failed and TestOps has no scoped plan for it.
…adPacer Clamping cost to the window's limit meant a single batch far bigger than the configured budget only ever reserved one window's delay, letting it blow through maxRequestsPerWindow/maxFilesPerWindow/maxBytesPerWindow by an arbitrary amount. Charging the full cost makes the next call wait proportionally longer instead.
…ready-acknowledged test result chunks uploadTestResults() chunks a batch and retries each chunk internally, but the plugin wrapped the whole call in a second, outer retry (UploadQueue). If a later chunk failed after an earlier one had already been accepted by TestOps, the outer retry re-invoked the same closure with the full, unfiltered batch - resending the already-accepted chunk. TestOps can't treat that resend as a no-op: a chunk's test results are upserted onto pre-existing "expected" rows only while those rows are still unfilled (status is null); once the first successful upload fills them in, the same uuids no longer match anything "expected" and the retried chunk creates brand new test_result rows instead - duplicating every test result from the chunk that already succeeded. uploadTestResults now reports each chunk's uploaded ids back via onChunkUploaded as it goes, and the plugin uses that to filter the batch passed into the retried closure down to what's still actually pending, so a retry (hot or at finalization) only ever resends what TestOps hasn't accepted yet.
…te budget attachmentByteLength only recognized Buffer and Blob, returning 0 for everything else - but resolved attachments flow through as a Readable stream in the normal reporting path, so maxBytesPerWindow never actually throttled real uploads, only the Buffer-backed ones exercised in tests. ResultFile already exposes getContentLength(), so capture it once when resolving the attachment and have attachmentByteLength prefer that over inspecting the content value.
…uest classifyError treated a canceled axios request (ERR_CANCELED/CanceledError) the same as a network failure with no response - ServiceTransient, i.e. retryable. httpRetry.ts's retryRequest already excludes cancellation from its own retry check; classifyError/withUploadRetry disagreed and would retry a request that was intentionally aborted.
…s a valid launchId, drop remaining comments - attachToLaunch's launchId check used a falsy test, so launchId: 0 would silently fall through to creating a new launch instead of attaching. - withUploadRetry's terminal check called isTerminalUploadError, whose isPermanentUploadError half is provably always false at that call site since shouldRetryUpload already returned true - simplified to the attempt-count check it actually reduces to. - #uploadAttachmentsForResult and the fixtures/attachments catch in #uploadChunkAttachmentsAndFixtures had the same three-way error-message formatting copy-pasted; extracted into #logResultUploadFailure. - Stripped the remaining explanatory comments and single-letter loop/ parameter names left in this branch's test files.
Dropping the inner retryRequest around the test-result chunk POST (to stop it double-retrying under the outer UploadQueue wrapper) also dropped Retry-After compliance for that call, since withUploadRetry's own backoff never read the header - a 429 with Retry-After: 30 would get hammered again after the default 500ms/1000ms/2000ms backoff instead of waiting the 30s TestOps asked for. withUploadRetry now reuses httpRetry.ts's retryAfterMs, which already unwraps KnownError/UnknownError via their captured axios .cause to read the header - not just raw AxiosErrors.
…wait Three gaps found reviewing the previous round of fixes: - Global attachments build their own AttachmentForUpload inline in plugin.ts, so they never got the contentLength the per-result resolver now carries - their byte pacing was still counting zero. - withUploadRetry honored Retry-After unbounded, so a server (or a misconfigured proxy) answering Retry-After: 3600 would stall the run for an hour per attempt. Capped at a minute, applied inside retryAfterMs so retryRequest gets the same bound. - ALLURE_LAUNCH_ID was accepted as anything that parses as a non-NaN number, so -5 and 1.5 reached attachToLaunch, while its sibling ALLURE_JOB_RUN_ID required a positive integer. Both now share toPositiveInteger, and it validates a launchId passed as an option too, not just the env fallback.
…ipped the upload createSession's POST is the one call in the upload path with no retry around it, and #upload() bails out of the whole cycle when it throws - without going through UploadQueue, so nothing gets deferred. At finalization that meant the entire final batch could be dropped by a single transient failure while stillPending stayed empty, so the autoclose guard didn't fire and the launch was closed and reported as finished with its results missing.
…of swallowing it syncLaunchCategories caught everything and returned silently, so results uploaded without their categories with nothing in the log to explain it. It now hands the failure back for the plugin to log, and returns early on a missing launch id instead of asserting one with a non-null assertion that only holds because of an invariant in another file.
…them on retry getNamedEnvFor scanned the whole named-env map comparing externalId, but the map is already keyed by exactly that - and it runs once per test result and once per fixture. The per-chunk retry closure also computed the "envs to create" set outside itself, so a retried chunk re-posted the named envs the failed attempt had already created.
…URE_ variable The decoded bundle deliberately overwrites ALLURE_ variables present in the current environment - the orchestrating step's snapshot is the authority. Nothing covered that, so the precedence was easy to flip while "fixing" what looked like an accidental clobber.
Two more flaky cases from packages/reader's xcresult suite, same as the existing entries in this rule - pass locally, flake in CI.
TestOpsClient#namedEnvs was never read anywhere; the class itself is not part of the package's public API, so it was unreachable from outside too.
resolutions.rules[].testCaseId matches against testResult.testCase.id, not the test result's own id (the report URL's hash). Fetched the result's JSON from the report and the two hashes I'd added turned out to be the same test: ddad4e5c... is the result id, d41ec9ab... is its actual testCase.id. Keeping only the one that matches.
allurerc.mjs's resolutions only gate the aggregated Allure report's quality gate (the Build report job) - they never touch vitest's own exit code, which is what actually reddens the macos-latest CI check. This test calls Apple's xcresulttool against a fixture bundle and its output format drifts across Xcode versions on the runner; it passes reliably locally, so skip it under CI only rather than always.
…one test CI reran with the previous single-test skip and a different test in the same suite failed the same way (empty result from xcresulttool), which means the flakiness isn't localized to one test - xcresulttool itself is intermittently returning nothing on this runner. Skipping individual tests one at a time as they come up isn't going to converge, and skipping the whole ~90-test suite in CI throws away real coverage for a transient issue. Retrying the whole suite up to twice under CI keeps that coverage and survives the flake; local runs are unaffected (retry 0).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Reworks
plugin-testopsto correlate CI-triggered uploads with an existing TestOps job run end-to-end, instead of always creating a new launch:launchIdoption, instead of always creating a new launchALLURE_CI_ENVpassthrough for pipeline steps that don't share env with the step that ran the plugingetRootURLto strip a trailing slash fromSYSTEM_COLLECTIONURItestops-planCLI command that fetches a job run's test plan from TestOpsFixes
Notes
Split the larger free functions out of
client.ts/plugin.tsintoutils/modules for readability, without changing theTestOpsClient/TestOpsPluginclass structure.