Skip to content

fix(data workflow tests): cancel running workflow executions before delete#2701

Closed
vincent-cognite wants to merge 24 commits into
masterfrom
CDF-28136-fix-workflow-integration-test-teardown
Closed

fix(data workflow tests): cancel running workflow executions before delete#2701
vincent-cognite wants to merge 24 commits into
masterfrom
CDF-28136-fix-workflow-integration-test-teardown

Conversation

@vincent-cognite

@vincent-cognite vincent-cognite commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Improve test harness for workflows:

  • Workflow API now rejects deletion while executions are still RUNNING (jazz-api#2424, CDF-28136).
  • Add integration test helpers to cancel running executions and wait for termination before deleting workflows.
  • Update workflow integration test teardown/cleanup paths to use safe delete, and cancel the retried execution left running by test_trigger_cancel_retry_workflow.

…ration tests

Workflow API now rejects deletion while executions are still running
(CDF-28136). Update workflow integration test teardown to cancel running
executions first so cleanup does not fail against the new guard.

Co-authored-by: Cursor <cursoragent@cursor.com>
@vincent-cognite vincent-cognite changed the title [CDF-28136] fix(tests): cancel running workflow executions before delete fix(data workflow tests): cancel running workflow executions before delete Jun 22, 2026
@codecov

codecov Bot commented Jun 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 65.09434% with 37 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.60%. Comparing base (e2152c0) to head (d6d0c38).

Files with missing lines Patch % Lines
tests/tests_integration/test_api/test_workflows.py 65.09% 37 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2701      +/-   ##
==========================================
- Coverage   93.68%   93.60%   -0.08%     
==========================================
  Files         503      503              
  Lines       50696    50778      +82     
==========================================
+ Hits        47494    47533      +39     
- Misses       3202     3245      +43     
Files with missing lines Coverage Δ
tests/tests_integration/test_api/test_workflows.py 85.04% <65.09%> (-5.38%) ⬇️

... and 6 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@vincent-cognite vincent-cognite marked this pull request as ready for review June 22, 2026 12:04
@vincent-cognite vincent-cognite requested review from a team as code owners June 22, 2026 12:04

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces helper functions _cancel_running_executions_for_workflow and _safe_delete_workflows in the integration tests to cancel active executions before deleting workflows, ensuring cleaner teardowns. The review feedback suggests improving robustness by wrapping execution cancellation in try...except CogniteAPIError blocks to prevent test flakiness if executions are already completed, and to properly respect the ignore_unknown_ids flag when attempting to cancel executions for non-existent workflows.

Comment thread tests/tests_integration/test_api/test_workflows.py Outdated
Comment thread tests/tests_integration/test_api/test_workflows.py Outdated
vincent-cognite and others added 4 commits June 22, 2026 14:09
… fixtures

Use test-scoped workflow executions so listing is not polluted by
executions created by other tests on the same session-scoped version.

Co-authored-by: Cursor <cursoragent@cursor.com>
… IDs

Ignore CogniteAPIError when cancelling executions that finish between
list and cancel, and skip cancellation for unknown workflows when
ignore_unknown_ids is enabled.

Co-authored-by: Cursor <cursoragent@cursor.com>
VerstraeteBert
VerstraeteBert previously approved these changes Jun 22, 2026
@vincent-cognite vincent-cognite self-assigned this Jun 22, 2026
@vincent-cognite vincent-cognite added the risk-review-ongoing Risk review is in progress label Jun 22, 2026
@vincent-cognite vincent-cognite added risk-review-ongoing Risk review is in progress and removed risk-review-ongoing Risk review is in progress labels Jun 22, 2026
@vincent-cognite vincent-cognite marked this pull request as draft June 23, 2026 13:16
auto-merge was automatically disabled June 23, 2026 13:16

Pull request was converted to draft

…h://github.com/cognitedata/cognite-sdk-python into CDF-28136-fix-workflow-integration-test-teardown
@vincent-cognite vincent-cognite marked this pull request as ready for review June 24, 2026 07:03

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request improves the stability and cleanup of workflow integration tests by introducing helper functions to cancel running executions before deleting workflows or versions, and isolating permanent workflows per pytest-xdist worker to prevent race conditions. The reviewer recommended optimizing the _safe_delete_workflows helper by batching workflow retrieval calls to avoid an N+1 query pattern, which will speed up the test cleanup process.

Comment thread tests/tests_integration/test_api/test_workflows.py Outdated
@vincent-cognite vincent-cognite marked this pull request as draft June 24, 2026 07:19
@vincent-cognite

Copy link
Copy Markdown
Contributor Author

too invasive and complicated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants