Fix child pipeline lookup - #1737
Conversation
…d-status handling in measurement.pipeline.ts, measurement.runner.ts, and ExecutionStatusService.ts respectively
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## feature/distributed-demo #1737 +/- ##
============================================================
- Coverage 98.37% 96.92% -1.45%
============================================================
Files 33 185 +152
Lines 2155 6152 +3997
Branches 0 1062 +1062
============================================================
+ Hits 2120 5963 +3843
- Misses 35 186 +151
- Partials 0 3 +3
... and 207 files with indirect coverage changes
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR fixes child pipeline resolution across the client by looking up downstream (child) pipelines via GitLab bridge jobs instead of assuming parentPipelineId + 1, and adjusts polling/cancellation and test coverage accordingly. It also introduces a small retry utility for read-only GitLab-loading flows and updates Playwright E2E tests/config to run with increased parallelism.
Changes:
- Add
getPipelineBridges+getChildPipelineIdto the backend interface and implement bridge-based child pipeline lookup for GitLab. - Update execution status polling, cancellation flows, and measurement pipeline lifecycle to use resolved child pipeline IDs (including retries while the child pipeline is not yet created).
- Add/adjust unit, integration, and E2E test coverage; introduce a shared E2E helper and update Playwright worker setup.
Reviewed changes
Copilot reviewed 35 out of 35 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| client/test/unit/route/digitaltwins/execution/executionStatusManager/testSetup.ts | Adds a reusable spy helper for getChildPipelineId in unit tests. |
| client/test/unit/route/digitaltwins/execution/executionStatusManager/childPipeline.test.ts | Updates unit tests to validate real child pipeline lookup (and retry when not yet available). |
| client/test/unit/model/backend/requestRetry.test.ts | Adds unit coverage for the new retry helper. |
| client/test/unit/model/backend/gitlab/model/ExecutionStatusService.test.ts | Updates backend mock expectations and adds cases for missing/errored child pipeline lookup. |
| client/test/unit/model/backend/gitlab/measure/measurement.testUtil.ts | Extends measurement backend mocks and active pipeline shape to include childPipelineId. |
| client/test/unit/model/backend/gitlab/measure/measurement.runner.test.ts | Tests that unload cancellation cancels discovered child pipelines and avoids guessing. |
| client/test/unit/model/backend/gitlab/measure/measurement.pipeline.test.ts | Updates pipeline lifecycle tests for child lookup + retry and new retry-on-rejection behavior. |
| client/test/unit/model/backend/gitlab/measure/measurement.lifecycle.test.ts | Updates mocks to remove the old getChildPipelineId helper from pipelineCore. |
| client/test/unit/model/backend/gitlab/measure/measurement.envSetup.ts | Updates environment setup mocks to remove the old getChildPipelineId helper. |
| client/test/unit/model/backend/gitlab/execution/pipelineCore.test.ts | Removes tests for the deleted getChildPipelineId(parent+1) behavior. |
| client/test/unit/model/backend/gitlab.test.ts | Adds unit tests for GitlabInstance.getChildPipelineId() bridge-based lookup. |
| client/test/unit/model/backend/digitalTwinUtil.test.ts | Extends API mock surface to include pipeline bridges. |
| client/test/unit/model/backend/digitalTwin/testSetup.ts | Extends GitLab instance mocks to include getChildPipelineId. |
| client/test/unit/model/backend/digitalTwin/stop.test.ts | Ensures stopping a child pipeline uses resolved child ID and handles “not yet created”. |
| client/test/unit/model/backend/backend.test.ts | Adds unit test for GitlabAPI.getPipelineBridges mapping behavior. |
| client/test/integration/route/digitaltwins/execution/executionStatusManager/childPipeline.test.tsx | Updates integration test to spy on getChildPipelineId during child polling. |
| client/test/e2e/tests/Measurement.test.ts | Increases timeouts for a measurement E2E test to match longer pipeline variability. |
| client/test/e2e/tests/execution.helpers.ts | Adds shared polling helper for execution-count-based waits in E2E tests. |
| client/test/e2e/tests/DigitalTwins.test.ts | Switches to the shared execution-count wait helper for “start then history” flow. |
| client/test/e2e/tests/ConcurrentExecution.test.ts | Updates concurrent-start flow to wait for persisted history entries via execution count. |
| client/test/mocks/mockBackendData.ts | Extends mocks for new backend API surface and child lookup. |
| client/src/route/digitaltwins/execution/executionStatusManager.ts | Resolves child pipeline IDs via backend bridge lookup and retries until available. |
| client/src/model/backend/util/requestRetry.ts | Introduces a generic retry helper with default attempts/delay. |
| client/src/model/backend/util/init.ts | Refactors instance initialization + wraps read-only loading flows with retries. |
| client/src/model/backend/util/digitalTwinPipelineExecution.ts | Updates stop pipeline resolution to use backend child pipeline lookup. |
| client/src/model/backend/state/ExecutionStatusService.ts | Updates execution status refresh to resolve child pipelines via backend lookup. |
| client/src/model/backend/interfaces/backendInterfaces.ts | Adds bridge and child-pipeline lookup APIs/types to backend interfaces. |
| client/src/model/backend/gitlab/measure/measurement.types.ts | Adds optional childPipelineId to active pipeline tracking. |
| client/src/model/backend/gitlab/measure/measurement.runner.ts | Cancels only known child pipelines during unload; removes guessed ID logic. |
| client/src/model/backend/gitlab/measure/measurement.pipeline.ts | Adds retrying child ID resolution and safer cancellation; adds backend init retry. |
| client/src/model/backend/gitlab/measure/constants.ts | Adds pipeline acceptance delay constant. |
| client/src/model/backend/gitlab/instance.ts | Implements getChildPipelineId via getPipelineBridges. |
| client/src/model/backend/gitlab/execution/pipelineCore.ts | Removes the old parent+1 child pipeline helper. |
| client/src/model/backend/gitlab/backend.ts | Implements getPipelineBridges using GitBeaker’s pipeline bridge jobs. |
| client/playwright.config.ts | Sets global workers to 3 and adjusts project dependencies for parallel E2E. |
| if (pipelineId == null) { | ||
| if (hasTimedOut(startTime)) { | ||
| await handleTimeout( | ||
| digitalTwin.DTName, | ||
| setButtonText, | ||
| setLogButtonDisabled, | ||
| dispatch, | ||
| executionId, | ||
| ); | ||
| return; | ||
| } | ||
| await delay(PIPELINE_POLL_INTERVAL); | ||
| await checkChildPipelineStatus({ | ||
| setButtonText, | ||
| digitalTwin, | ||
| setLogButtonDisabled, | ||
| dispatch, | ||
| startTime, | ||
| executionId, | ||
| }); | ||
| return; |
| for (;;) { | ||
| const childPipelineId = await backend.getChildPipelineId( | ||
| projectId, | ||
| parentPipelineId, | ||
| ); |
| await backend.api.cancelPipeline(projectId, pipelineId); | ||
| const childPipelineId = await backend | ||
| .getChildPipelineId(projectId, pipelineId) | ||
| .catch(() => null); | ||
| await cancelChildPipeline(backend, projectId, childPipelineId); |
| await page.waitForTimeout(DEBOUNCE_TIME); // NOSONAR | ||
| await startButton.click(); | ||
| await waitForNewExecution(historyButton, previousCount); | ||
| await waitForExecutionCount(historyButton, 1); |
| await waitForExecutionCount(historyButton, 1); | ||
| await expect(startButton).toBeEnabled({ | ||
| timeout: EXECUTION_START_TIMEOUT, | ||
| }); | ||
| await startButton.click(); | ||
| await waitForExecutionCount(historyButton, 2); |
|
@atomicgamedeveloper please see the review from claude Full Code Review — PR #1737 "Fix child pipeline lookup"INTO-CPS-Association/DTaaS — atomicgamedeveloper:pipeline-child-id-fix → feature/distributed-demo SummaryReplaces the previous
Files reviewedConfirmed from diff: Correctness / design
Type safety / API boundary
Test/CI changes
RecommendationThe core fix (bridge-based child pipeline resolution) is correct and
|
…GitLab errors safely; warn when a job cannot be stopped; make shared job-history checks reliable; add comments otherwise.
…developer/DTaaS into pipeline-child-id-fix
|
|
@atomicgamedeveloper please see this updates review Full Code Review — PR #1737 "Fix child pipeline lookup" (Updated)INTO-CPS-Association/DTaaS — atomicgamedeveloper:pipeline-child-id-fix → feature/distributed-demo What changed since the last reviewThe PR now includes four additional commits beyond what was previously
The PR description now explicitly states the retry policy: a failed CodeCov confirms SummaryReplaces the previous
Files reviewedConfirmed from diff: Correctness / design
Type safety / API boundary
Test/CI changes
RecommendationThe core fix (bridge-based child pipeline resolution) remains correct
Do tag me when you have reaolved the comments by both copilot and claude. Thanks. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 35 out of 35 changed files in this pull request and generated no new comments.
Suppressed comments (3)
client/src/route/digitaltwins/execution/executionStatusManager.ts:255
checkChildPipelineStatusre-fetches the child pipeline ID on every poll. This can unnecessarily hit GitLab, and a transientgetChildPipelineIdfailure will currently reject the whole polling loop. Cache the child pipeline ID once discovered and treat lookup errors as retryable.
while (true) {
const pipelineId = await digitalTwin.backend.getChildPipelineId(
digitalTwin.backend.getProjectId(),
parentPipelineId,
);
client/src/route/digitaltwins/execution/executionStatusManager.ts:276
getPipelineStatuserrors (e.g. transient network failures) currently bubble up and abort status polling. Since this is part of a polling loop, treat request failures as retryable (keep polling until timeout) rather than crashing the status manager.
const pipelineStatus = await digitalTwin.backend.getPipelineStatus(
digitalTwin.backend.getProjectId(),
pipelineId,
);
client/src/route/digitaltwins/execution/executionStatusManager.ts:1
- File-wide
eslint-disablecan mask unrelated issues elsewhere in this module. Prefer scoping the suppression to the specific polling loop (or a small block) so other rules still apply to the rest of the file.
/* eslint-disable no-await-in-loop, no-continue */
Pull Request Template
Title
Fix child pipeline lookup
Type of Change
Description
Fixes #1705.
Use GitLab bridge jobs to resolve the actual child pipeline instead of assuming it is the next pipeline ID.
The previously sequential E2E pipeline tests now run with three workers. Read-only GitLab loading requests also retry temporary failures safely.
Testing
Added unit and integration coverage for child pipeline lookup and retrying.
For a failed read-only GitLab request, the app waits 250 ms and retries up to two times. Requests that start a pipeline are never retried, so this cannot create duplicate jobs.
Impact
Status checks, logs, and cancellation use the correct child pipeline when other pipelines run at the same time. E2E pipeline tests run in parallel while handling temporary GitLab loading failures.
Additional Information
Checklist
existing code.