Skip to content

test(orchestrator): close L1 gaps for cloud-event execute RHIDP-16048 - #4170

Open
rostalan wants to merge 3 commits into
redhat-developer:mainfrom
rostalan:rhidp-16048-l1-ce-unit-tests
Open

test(orchestrator): close L1 gaps for cloud-event execute RHIDP-16048#4170
rostalan wants to merge 3 commits into
redhat-developer:mainfrom
rostalan:rhidp-16048-l1-ce-unit-tests

Conversation

@rostalan

@rostalan rostalan commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Cover compiled start.stateName happy path, CE call wiring, and non-object workflowdata unwrap.

Cover compiled start.stateName happy path, CE call wiring, and non-object workflowdata unwrap.

Co-authored-by: Cursor <cursoragent@cursor.com>
@rhdh-gh-app

rhdh-gh-app Bot commented Aug 5, 2026

Copy link
Copy Markdown

Missing Changesets

The following package(s) are changed by this PR but do not have a changeset:

  • @red-hat-developer-hub/backstage-plugin-orchestrator-backend

See CONTRIBUTING.md for more information about how to add changesets.

Changed Packages

Package Name Package Path Changeset Bump Current Version
@red-hat-developer-hub/backstage-plugin-orchestrator-backend workspaces/orchestrator/plugins/orchestrator-backend none v8.11.0

@rhdh-qodo-merge

Copy link
Copy Markdown

PR Summary by Qodo

Add unit tests for compiled start.stateName and CloudEvent execute wiring

🧪 Tests 🕐 10-20 Minutes

Grey Divider

AI Description

• Add coverage for compiled workflows that use start.stateName.
• Assert executeWorkflowAsCloudEvent wiring and payload shaping in v2 execute tests.
• Verify CloudEvent data defaults when workflowdata is missing or not an object.
Diagram

graph TD
  TestV2["v2.test.ts"] --> ApiV2["API v2 execute"] --> OrchSvc["OrchestratorService mock"] --> SF["SonataFlowService"] --> Kafka["Kafka producer"]
  TestSF["SonataFlowService.test.ts"] --> SF
  TestUtils["test-utils.ts"] --> TestV2
Loading
High-Level Assessment

The PR’s approach—adding targeted unit tests plus a small fixture helper for the compiled start.stateName YAML variant—is the most direct way to close the identified L1 coverage gaps without changing production code.

Files changed (3) +137 / -0

Tests (3) +137 / -0
SonataFlowService.test.tsAdd test for CloudEvent data defaulting when workflowdata is invalid +43/-0

Add test for CloudEvent data defaulting when workflowdata is invalid

• Adds a unit test ensuring executeWorkflowAsCloudEvent publishes a CloudEvent with an empty/safe data shape when inputData.workflowdata is missing or not an object. The test verifies the produced Kafka message JSON contains the correlation context attribute while omitting workflowdata.

workspaces/orchestrator/plugins/orchestrator-backend/src/service/SonataFlowService.test.ts

test-utils.tsAdd workflow fixture generator for compiled start.stateName form +14/-0

Add workflow fixture generator for compiled start.stateName form

• Introduces a helper that modifies the base event-type workflow YAML to use the compiled-workflow format (start.stateName) instead of a string start. This enables tests to cover the happy path for compiled workflow definitions.

workspaces/orchestrator/plugins/orchestrator-backend/src/service/api/test-utils.ts

v2.test.tsExpand v2 execute tests to assert CloudEvent call wiring and compiled start handling +80/-0

Expand v2 execute tests to assert CloudEvent call wiring and compiled start handling

• Strengthens the event-type executeWorkflow test by asserting executeWorkflowAsCloudEvent is called with expected definition metadata, context attribute, and inputData fields. Adds a new test case validating the same behavior when the workflow definition uses compiled start.stateName.

workspaces/orchestrator/plugins/orchestrator-backend/src/service/api/v2.test.ts

@rhdh-qodo-merge

rhdh-qodo-merge Bot commented Aug 5, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Unsafe mock call indexing ✓ Resolved 🐞 Bug ☼ Reliability
Description
The new SonataFlowService test reads sendMock.mock.calls[0] without first asserting sendMock was
called, which can fail with an unhelpful TypeError if send() is not invoked (or behavior changes)
rather than a clear expectation failure.
Code

workspaces/orchestrator/plugins/orchestrator-backend/src/service/SonataFlowService.test.ts[R333-335]

+      const { messages } = sendMock.mock.calls[0][0];
+      const parsed = JSON.parse(messages[0].value);
+
Relevance

●●● Strong

Repo previously accepted refactors to make Kafka send assertions safer/clearer in this same test
file.

PR-#2896

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The newly added test inspects sendMock.mock.calls[0] directly, while an adjacent earlier test
demonstrates the safer pattern of asserting the mock was called first.

workspaces/orchestrator/plugins/orchestrator-backend/src/service/SonataFlowService.test.ts[245-295]
workspaces/orchestrator/plugins/orchestrator-backend/src/service/SonataFlowService.test.ts[299-340]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The test dereferences `sendMock.mock.calls[0][0]` without checking that `sendMock` was called. If the implementation exits early or changes, this test can throw a TypeError while inspecting the mock instead of reporting that `send()` was not called.

### Issue Context
The previous CloudEvent test in the same file uses `expect(sendMock).toHaveBeenCalled()` before inspecting `mock.calls`, which provides clearer failure output.

### Fix Focus Areas
- workspaces/orchestrator/plugins/orchestrator-backend/src/service/SonataFlowService.test.ts[299-340]

### Suggested fix
Add an assertion before indexing:
- `expect(sendMock).toHaveBeenCalledTimes(1);`
Optionally, also assert the shape of the argument:
- `expect(sendMock).toHaveBeenCalledWith(expect.objectContaining({ topic: 'workflowEventType', messages: expect.any(Array) }))`

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Unchecked YAML replace fixture ✓ Resolved 🐞 Bug ⚙ Maintainability
Description
generateTestWorkflowInfoForEventypeWithStartStateName rewrites the YAML via a literal string replace
without verifying the pattern matched, so a small fixture formatting change can silently stop
exercising the intended start.stateName code path.
Code

workspaces/orchestrator/plugins/orchestrator-backend/src/service/api/test-utils.ts[R360-363]

+    source: (base.source as string).replace(
+      'start: listenToLock',
+      'start:\n  stateName: listenToLock',
+    ),
Relevance

●● Moderate

Defensive check on string replace improves test robustness, but no close precedent found for
fixture-rewrite helpers.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The helper performs an unchecked substring replace, and the base fixture currently contains the
literal start: listenToLock, meaning future formatting changes can cause a silent no-op and reduce
test coverage of the compiled start.stateName branch.

workspaces/orchestrator/plugins/orchestrator-backend/src/service/api/test-utils.ts[272-321]
workspaces/orchestrator/plugins/orchestrator-backend/src/service/api/test-utils.ts[353-365]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`generateTestWorkflowInfoForEventypeWithStartStateName` uses `String.replace(...)` to convert `start: listenToLock` into a compiled-workflow shape (`start:\n  stateName: ...`). If the base YAML changes slightly (quotes/indentation/spacing), the replace can do nothing and the compiled-start test will unintentionally fall back to the non-compiled `start` form.

### Issue Context
This helper is intended to ensure the v2 event-type execution path is validated against compiled workflows, which use `start.stateName`.

### Fix Focus Areas
- workspaces/orchestrator/plugins/orchestrator-backend/src/service/api/test-utils.ts[353-365]

### Suggested fix
Prefer one of:
1) Parse and modify the YAML structurally:
  - `const doc = load(base.source as string)`
  - set `doc.start = { stateName: 'listenToLock' }`
  - `source: dump(doc)`

2) Keep string replacement but assert it succeeded:
  - compute `newSource`
  - if `newSource === base.source`, `throw new Error('Failed to rewrite start to start.stateName')` (or at least `expect` in the test).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context used
✅ Compliance rules (platform): 11 rules
✅ Cross-repo context
  Not relevant to this PR: redhat-developer/rhdh
  Not relevant to this PR: redhat-developer/rhdh-chart
  Not relevant to this PR: redhat-developer/rhdh-operator
  Not relevant to this PR: redhat-developer/rhdh-local

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

@rhdh-qodo-merge rhdh-qodo-merge Bot added the Tests label Aug 5, 2026
@codecov

codecov Bot commented Aug 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.12%. Comparing base (0f32982) to head (b421989).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4170   +/-   ##
=======================================
  Coverage   58.11%   58.12%           
=======================================
  Files        2422     2422           
  Lines       96484    96493    +9     
  Branches    26885    26869   -16     
=======================================
+ Hits        56075    56084    +9     
- Misses      38914    38917    +3     
+ Partials     1495     1492    -3     
Flag Coverage Δ *Carryforward flag
adoption-insights 84.55% <ø> (ø) Carriedforward from 6282e29
ai-integrations 69.76% <ø> (ø) Carriedforward from 6282e29
app-defaults 69.79% <ø> (ø) Carriedforward from 6282e29
augment 46.67% <ø> (ø) Carriedforward from 6282e29
boost 76.77% <ø> (ø) Carriedforward from 6282e29
bulk-import 72.56% <ø> (ø) Carriedforward from 6282e29
cost-management 13.55% <ø> (ø) Carriedforward from 6282e29
dcm 60.72% <ø> (ø) Carriedforward from 6282e29
extensions 56.59% <ø> (ø) Carriedforward from 6282e29
global-floating-action-button 71.18% <ø> (ø) Carriedforward from 6282e29
global-header 66.50% <ø> (ø) Carriedforward from 6282e29
homepage 47.59% <ø> (ø) Carriedforward from 6282e29
install-dynamic-plugins 59.95% <ø> (ø) Carriedforward from 6282e29
intelligent-assistant 74.59% <ø> (ø) Carriedforward from 6282e29
konflux 91.98% <ø> (ø) Carriedforward from 6282e29
lightspeed 69.02% <ø> (ø) Carriedforward from 6282e29
mcp-integrations 83.40% <ø> (ø) Carriedforward from 6282e29
orchestrator 66.91% <100.00%> (+0.04%) ⬆️
quickstart 63.74% <ø> (ø) Carriedforward from 6282e29
sandbox 79.56% <ø> (ø) Carriedforward from 6282e29
scorecard 85.98% <ø> (ø) Carriedforward from 6282e29
theme 88.77% <ø> (ø) Carriedforward from 6282e29
translations 5.12% <ø> (ø) Carriedforward from 6282e29
x2a 79.20% <ø> (ø) Carriedforward from 6282e29

*This pull request uses carry forward flags. Click here to find out more.


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0f32982...b421989. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

rostalan and others added 2 commits August 5, 2026 11:51
Assert Kafka send was called before inspecting mock args, and fail if the compiled start.stateName fixture rewrite misses.

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

sonarqubecloud Bot commented Aug 5, 2026

Copy link
Copy Markdown

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant