You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@texera.apache.org by "aglinxinyuan (via GitHub)" <gi...@apache.org> on 2026/07/22 05:32:44 UTC
[PR] test(frontend): extend WorkflowActionService unit test coverage [texera]
aglinxinyuan opened a new pull request, #6770:
URL: https://github.com/apache/texera/pull/6770
### What changes were proposed in this PR?
Extends `workflow-action.service.spec.ts` with 18 tests (37 -> 55) for `WorkflowActionService`, covering the workflow-modification lock, `addPort` input/output paths and all three error branches (plus the portID-conflict loop), comment-box/comment add-edit-delete, `deleteOperatorsAndLinks` (source- and target-matched links + Set dedup), auto-layout comment-box positioning, the result-panel open/close streams, `setWorkflowSettings` same-reference short-circuit, `getWorkflowContent`/`getWorkflow` assembly, `reloadWorkflow`, and the drag-together handler. Coverage 88% -> ~100% (63 -> 0 uncovered). No existing tests modified.
### Any related issues, documentation, discussions?
Closes #6766.
### How was this PR tested?
`ng test --include='**/workflow-action.service.spec.ts'` -> 55/55 passing. `yarn format:ci` passes.
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 4.8 [1M context])
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(frontend): extend WorkflowActionService unit test coverage [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #6770:
URL: https://github.com/apache/texera/pull/6770#issuecomment-5042227525
<!-- texera-reviewer-suggestion -->
### Automated Reviewer Suggestions
Based on the `git blame` history of the changed files, we recommend the following reviewers:
- No candidates found from `git blame` history.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(frontend): extend WorkflowActionService unit test coverage [texera]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #6770:
URL: https://github.com/apache/texera/pull/6770#discussion_r3627713525
##########
frontend/src/app/workspace/service/workflow-graph/model/workflow-action.service.spec.ts:
##########
@@ -588,4 +588,270 @@ describe("WorkflowActionService", () => {
expect(service.getCenterPoint()).toEqual({ x: 100, y: 250 });
});
+
+ it("should leave the center point at its default when there are no operators", () => {
+ const before = service.getCenterPoint();
+
+ service.calculateTopLeftOperatorPosition();
+
+ expect(service.getCenterPoint()).toEqual(before);
+ expect(service.getCenterPoint()).toEqual({ x: 0, y: 0 });
+ });
+
+ it("should toggle the workflow-modification lock and emit each state on the stream", () => {
+ const emitted: boolean[] = [];
+ service.getWorkflowModificationEnabledStream().subscribe(v => emitted.push(v));
+
+ // BehaviorSubject replays the initial "enabled" state on subscription
+ expect(service.checkWorkflowModificationEnabled()).toBeTruthy();
+
+ service.disableWorkflowModification();
+ expect(service.checkWorkflowModificationEnabled()).toBeFalsy();
+
+ // enable only takes effect when previously disabled (and workflow not readonly)
+ service.enableWorkflowModification();
+ expect(service.checkWorkflowModificationEnabled()).toBeTruthy();
+
+ expect(emitted).toEqual([true, false, true]);
+ });
+
+ it("should expose the underlying joint graph instance", () => {
+ expect(service.getJointGraph()).toBe(jointGraph);
+ });
+
+ it("should resolve a conflicting port ID when adding a dynamic input port", () => {
+ // inputPorts already contains "input-1" but has length 1, so the first candidate
+ // portID ("input-" + 1) collides and the loop must bump the suffix to "input-2".
+ const gapOp: OperatorPredicate = {
+ ...mockMultiInputOutputPredicate,
+ operatorID: "gap-op",
+ dynamicInputPorts: true,
+ inputPorts: [{ portID: "input-1" }],
+ };
+ service.addOperator(gapOp, mockPoint);
+
+ service.addPort(gapOp.operatorID, true);
+
+ const portIDs = texeraGraph.getOperator(gapOp.operatorID).inputPorts.map(p => p.portID);
+ expect(portIDs).toContain("input-2");
+ expect(portIDs).not.toContain("input-1input-1");
+ });
+
+ it("should add a dynamic output port to an operator that allows them", () => {
+ const dynOutOp: OperatorPredicate = {
+ ...mockMultiInputOutputPredicate,
+ operatorID: "dyn-out",
+ dynamicOutputPorts: true,
+ };
+ service.addOperator(dynOutOp, mockPoint);
+ const beforeOutputs = texeraGraph.getOperator(dynOutOp.operatorID).outputPorts.length;
+
+ service.addPort(dynOutOp.operatorID, false);
+
+ expect(texeraGraph.getOperator(dynOutOp.operatorID).outputPorts.length).toEqual(beforeOutputs + 1);
+ });
+
+ it("should throw when adding an output port to an operator without dynamic output ports", () => {
+ const noOutOp: OperatorPredicate = { ...mockMultiInputOutputPredicate, operatorID: "no-out" };
+ service.addOperator(noOutOp, mockPoint);
+
+ expect(() => service.addPort(noOutOp.operatorID, false)).toThrowError(
+ new RegExp("does not have dynamic output ports")
+ );
+ });
+
+ it("should throw when specifying disallowMultiInputs on an output port", () => {
+ const dynOutOp: OperatorPredicate = {
+ ...mockMultiInputOutputPredicate,
+ operatorID: "dyn-out-2",
+ dynamicOutputPorts: true,
+ };
+ service.addOperator(dynOutOp, mockPoint);
+
+ expect(() => service.addPort(dynOutOp.operatorID, false, true)).toThrowError(
+ new RegExp("disallowMultiInputs property of an output port should not be specified")
+ );
+ });
+
+ it("should add a comment box together with its initial comments", () => {
+ const comment = { content: "hello", creationTime: "2020-01-01", creatorName: "me", creatorID: 1 };
+ const box = { ...mockCommentBox, commentBoxID: "commentBox-with-comment", comments: [comment] };
+
+ service.addCommentBox(box);
+
+ expect(texeraGraph.hasCommentBox("commentBox-with-comment")).toBeTruthy();
+ expect(texeraGraph.getCommentBox("commentBox-with-comment").comments.length).toEqual(1);
+ expect(texeraGraph.getCommentBox("commentBox-with-comment").comments[0].content).toEqual("hello");
+ });
+
+ it("should delete a comment box and throw when deleting a non-existing one", () => {
+ const box = { ...mockCommentBox, commentBoxID: "commentBox-del" };
+ service.addCommentBox(box);
+ expect(texeraGraph.hasCommentBox("commentBox-del")).toBeTruthy();
+
+ service.deleteCommentBox("commentBox-del");
+ expect(texeraGraph.hasCommentBox("commentBox-del")).toBeFalsy();
+
+ expect(() => service.deleteCommentBox("commentBox-missing")).toThrowError(new RegExp("does not exist"));
+ });
+
+ it("should add, edit, and delete a comment inside a comment box", () => {
+ const box = { ...mockCommentBox, commentBoxID: "commentBox-cmt" };
+ service.addCommentBox(box);
+ const comment = { content: "first", creationTime: "2020-02-02", creatorName: "author", creatorID: 7 };
+
+ service.addComment(comment, "commentBox-cmt");
+ expect(texeraGraph.getCommentBox("commentBox-cmt").comments.length).toEqual(1);
+
+ service.editComment(7, "2020-02-02", "commentBox-cmt", "edited");
+ expect(texeraGraph.getCommentBox("commentBox-cmt").comments[0].content).toEqual("edited");
+
+ service.deleteComment(7, "2020-02-02", "commentBox-cmt");
+ expect(texeraGraph.getCommentBox("commentBox-cmt").comments.length).toEqual(0);
+ });
+
+ it("should delete multiple operators along with their connecting links", () => {
+ service.addOperator(mockScanPredicate, mockPoint);
+ service.addOperator(mockSentimentPredicate, mockPoint);
+ service.addOperator(mockResultPredicate, mockPoint);
+ service.addLink(mockScanSentimentLink);
+ service.addLink(mockSentimentResultLink);
+
+ // duplicate IDs verify the internal Set de-duplication path too
+ service.deleteOperatorsAndLinks([
+ mockScanPredicate.operatorID,
+ mockSentimentPredicate.operatorID,
+ mockScanPredicate.operatorID,
+ ]);
+
+ expect(texeraGraph.hasOperator(mockScanPredicate.operatorID)).toBeFalsy();
+ expect(texeraGraph.hasOperator(mockSentimentPredicate.operatorID)).toBeFalsy();
+ expect(texeraGraph.hasOperator(mockResultPredicate.operatorID)).toBeTruthy();
+ expect(texeraGraph.getAllLinks().length).toEqual(0);
+ });
+
+ it("should delete a downstream operator and drop links matched by their target", () => {
+ service.addOperator(mockScanPredicate, mockPoint);
+ service.addOperator(mockResultPredicate, mockPoint);
+ service.addLink(mockScanResultLink); // scan -> result
+
+ // deleting only the target operator: the link matches on its target, not its source,
+ // exercising the right-hand operand of the link filter's OR condition
+ service.deleteOperatorsAndLinks([mockResultPredicate.operatorID]);
+
+ expect(texeraGraph.hasOperator(mockScanPredicate.operatorID)).toBeTruthy();
+ expect(texeraGraph.hasOperator(mockResultPredicate.operatorID)).toBeFalsy();
+ expect(texeraGraph.getAllLinks().length).toEqual(0);
+ });
+
+ it("should record comment-box positions during auto layout", () => {
+ service.addOperator(mockScanPredicate, mockPoint);
+ const box = { ...mockCommentBox, commentBoxID: "commentBox-layout" };
+ service.addCommentBox(box);
+
+ service.autoLayoutWorkflow();
+
+ expect(texeraGraph.sharedModel.elementPositionMap.get("commentBox-layout")).toBeDefined();
+ });
+
+ it("should emit open and close events on the result-panel stream", () => {
+ const events: boolean[] = [];
+ service.resultPanelOpen$.subscribe(v => events.push(v));
+
+ service.openResultPanel();
+ service.closeResultPanel();
+
+ expect(events).toEqual([true, false]);
+ });
+
+ it("should not replace the workflow settings object when given the same reference", () => {
+ const current = service.getWorkflowSettings();
+
+ service.setWorkflowSettings(current);
+
+ expect(service.getWorkflowSettings()).toBe(current);
+ });
+
+ it("should assemble workflow content and the full workflow from graph state", () => {
+ service.addOperator(mockScanPredicate, { x: 10, y: 20 });
+ service.addOperator(mockResultPredicate, { x: 30, y: 40 });
+ service.addLink(mockScanResultLink);
+ service.setWorkflowName("Content WF");
+
+ const content = service.getWorkflowContent();
+ expect(content.operators.map(o => o.operatorID).sort()).toEqual([
+ mockScanPredicate.operatorID,
+ mockResultPredicate.operatorID,
+ ]);
+ expect(content.links.length).toEqual(1);
+ expect(content.operatorPositions[mockScanPredicate.operatorID]).toEqual({ x: 10, y: 20 });
+ expect(content.operatorPositions[mockResultPredicate.operatorID]).toEqual({ x: 30, y: 40 });
+ expect(content.settings).toEqual(service.getWorkflowSettings());
+
+ const workflow = service.getWorkflow();
+ expect(workflow.name).toEqual("Content WF");
+ expect(workflow.content.operators.length).toEqual(2);
+ expect(workflow.content.links.length).toEqual(1);
+ });
+
+ it("should clear pre-existing comment boxes and fall back to default settings when reloading", () => {
+ service.addCommentBox({ ...mockCommentBox, commentBoxID: "commentBox-old" });
+ expect(texeraGraph.hasCommentBox("commentBox-old")).toBeTruthy();
+
+ const workflow: Workflow = {
+ ...DEFAULT_WORKFLOW,
+ content: {
+ operators: [mockScanPredicate],
+ operatorPositions: { [mockScanPredicate.operatorID]: mockPoint },
+ links: [],
+ commentBoxes: [],
+ settings: undefined as any,
+ },
+ };
+
+ service.reloadWorkflow(workflow, false, false);
+
+ expect(texeraGraph.hasCommentBox("commentBox-old")).toBeFalsy();
+ expect(texeraGraph.hasOperator(mockScanPredicate.operatorID)).toBeTruthy();
+ // settings was undefined in the reloaded content, so defaults are applied
+ expect(service.getWorkflowSettings().dataTransferBatchSize).toEqual(100);
+ expect(service.getWorkflowSettings().executionMode).toEqual(ExecutionMode.PIPELINED);
+ });
Review Comment:
This reloadWorkflow test hard-codes default settings values (100/PIPELINED) and also doesn’t prove the fallback path is exercised, because the service starts with those defaults already. This can make the test brittle (if the mock config defaults change) and allow a regression where reload keeps the previous settings instead of applying defaults.
Consider setting workflow settings to a non-default value before reload, then asserting the post-reload settings equal the injected config defaults.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(frontend): extend WorkflowActionService unit test coverage [texera]
Posted by "aglinxinyuan (via GitHub)" <gi...@apache.org>.
aglinxinyuan merged PR #6770:
URL: https://github.com/apache/texera/pull/6770
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(frontend): extend WorkflowActionService unit test coverage [texera]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #6770:
URL: https://github.com/apache/texera/pull/6770#issuecomment-5042765019
## [Codecov](https://app.codecov.io/gh/apache/texera/pull/6770?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 75.12%. Comparing base ([`91a701b`](https://app.codecov.io/gh/apache/texera/commit/91a701b7c1dc16f3a896971ddef4c1859a50e51f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`83ef89b`](https://app.codecov.io/gh/apache/texera/commit/83ef89b38ea0e32fd8c9de0769bd580b8b369718?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on main.
:white_check_mark: All tests successful. No failed tests found.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #6770 +/- ##
=========================================
Coverage 75.12% 75.12%
Complexity 3449 3449
=========================================
Files 1160 1160
Lines 45807 45807
Branches 5071 5071
=========================================
Hits 34412 34412
Misses 9777 9777
Partials 1618 1618
```
| [Flag](https://app.codecov.io/gh/apache/texera/pull/6770/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | *Carryforward flag |
|---|---|---|---|
| [access-control-service](https://app.codecov.io/gh/apache/texera/pull/6770/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `70.00% <ø> (ø)` | | Carriedforward from [1fa2f56](https://app.codecov.io/gh/apache/texera/commit/1fa2f56586a894882cd97734b2f6bdab09829f16?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [agent-service](https://app.codecov.io/gh/apache/texera/pull/6770/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `76.76% <ø> (ø)` | | Carriedforward from [1fa2f56](https://app.codecov.io/gh/apache/texera/commit/1fa2f56586a894882cd97734b2f6bdab09829f16?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [amber](https://app.codecov.io/gh/apache/texera/pull/6770/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `67.19% <ø> (ø)` | | Carriedforward from [1fa2f56](https://app.codecov.io/gh/apache/texera/commit/1fa2f56586a894882cd97734b2f6bdab09829f16?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [computing-unit-managing-service](https://app.codecov.io/gh/apache/texera/pull/6770/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `20.49% <ø> (ø)` | | Carriedforward from [1fa2f56](https://app.codecov.io/gh/apache/texera/commit/1fa2f56586a894882cd97734b2f6bdab09829f16?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [config-service](https://app.codecov.io/gh/apache/texera/pull/6770/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `66.66% <ø> (ø)` | | Carriedforward from [1fa2f56](https://app.codecov.io/gh/apache/texera/commit/1fa2f56586a894882cd97734b2f6bdab09829f16?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [file-service](https://app.codecov.io/gh/apache/texera/pull/6770/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `67.21% <ø> (ø)` | | Carriedforward from [1fa2f56](https://app.codecov.io/gh/apache/texera/commit/1fa2f56586a894882cd97734b2f6bdab09829f16?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [frontend](https://app.codecov.io/gh/apache/texera/pull/6770/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `79.22% <ø> (ø)` | | Carriedforward from [1fa2f56](https://app.codecov.io/gh/apache/texera/commit/1fa2f56586a894882cd97734b2f6bdab09829f16?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [notebook-migration-service](https://app.codecov.io/gh/apache/texera/pull/6770/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `78.94% <ø> (ø)` | | Carriedforward from [1fa2f56](https://app.codecov.io/gh/apache/texera/commit/1fa2f56586a894882cd97734b2f6bdab09829f16?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [pyamber](https://app.codecov.io/gh/apache/texera/pull/6770/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `92.15% <ø> (ø)` | | Carriedforward from [1fa2f56](https://app.codecov.io/gh/apache/texera/commit/1fa2f56586a894882cd97734b2f6bdab09829f16?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [workflow-compiling-service](https://app.codecov.io/gh/apache/texera/pull/6770/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.14% <ø> (ø)` | | Carriedforward from [1fa2f56](https://app.codecov.io/gh/apache/texera/commit/1fa2f56586a894882cd97734b2f6bdab09829f16?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
*This pull request uses carry forward flags. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/texera/pull/6770?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(frontend): extend WorkflowActionService unit test coverage [texera]
Posted by "aglinxinyuan (via GitHub)" <gi...@apache.org>.
aglinxinyuan closed pull request #6770: test(frontend): extend WorkflowActionService unit test coverage
URL: https://github.com/apache/texera/pull/6770
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org