This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/texera.git
The following commit(s) were added to refs/heads/main by this push:
new c16e152f2f test(validation): cover
ValidationWorkflowService.combineValidation (#8604)
c16e152f2f is described below
commit c16e152f2f2b7b239a8660b77b9b1e539c8c2236
Author: Suyash Jain <[email protected]>
AuthorDate: Sun Sep 20 18:04:05 2026 +0000
test(validation): cover ValidationWorkflowService.combineValidation (#8604)
### What changes were proposed in this PR?
`ValidationWorkflowService.combineValidation` is what folds the
per-check results (JSON-schema validation and operator-connection
validation) into the single verdict the workflow editor acts on:
```ts
public static combineValidation(...validations: Validation[]): Validation
```
It had no direct test. This PR adds one, covering the three decisions
the implementation actually makes:
- **Validity is an AND** across all validations, independent of argument
order.
- **Messages are merged only from the validations that failed.** The
merge is guarded on `validation.isValid` rather than on the `messages`
key being absent, so a stray `messages` field on something reporting
`isValid: true` is dropped rather than leaking into the combined result.
- **A later failure wins** when two failing validations use the same
message key, since the merge is a spread in argument order.
Two shape details are asserted as well, because call sites depend on
them:
- The valid branch returns `{ isValid }` only — it omits `messages`
entirely rather than returning an empty map, so reading `messages` off a
valid result yields `undefined`.
- A no-argument call is vacuously valid.
No production code is changed; this is a test-only PR.
### Any related issues, documentation, discussions?
Closes #6684
### How was this PR tested?
Nine cases were added to `validation-workflow.service.spec.ts` in a new
top-level `describe`. `combineValidation` is static, so the block needs
no `TestBed` setup.
| Case | Asserts |
| --- | --- |
| no arguments | vacuously valid, `messages` is `undefined` |
| all valid | result is exactly `{ isValid: true }` |
| one invalid | invalid, carries that validation's messages |
| invalid first / invalid last | invalid either way (order independence)
|
| several invalid | message maps are merged |
| key collision | the later failure's message wins |
| stray `messages` on a valid validation | dropped from the result |
| invalid with an empty message map | invalid, `messages` is `{}` |
| any combination | the input validations are not mutated |
```
npx ng test --watch=false --include="**/validation-workflow.service.spec.ts"
# Test Files 1 passed (1)
# Tests 19 passed (19) <- 10 existing + 9 added here
```
`npx prettier --check` reports no diff on the changed file.
### Was this PR authored or co-authored using generative AI tooling?
Yes, partially. I (Suyash Jain) worked on this PR together with Claude
Code as a pair-programming assistant. The added specs were run locally
against the existing suite before opening this PR.
---
.../validation/validation-workflow.service.spec.ts | 92 +++++++++++++++++++++-
1 file changed, 91 insertions(+), 1 deletion(-)
diff --git
a/frontend/src/app/workspace/service/validation/validation-workflow.service.spec.ts
b/frontend/src/app/workspace/service/validation/validation-workflow.service.spec.ts
index a755fa488a..0ccb522afd 100644
---
a/frontend/src/app/workspace/service/validation/validation-workflow.service.spec.ts
+++
b/frontend/src/app/workspace/service/validation/validation-workflow.service.spec.ts
@@ -18,7 +18,7 @@
*/
import { inject, TestBed } from "@angular/core/testing";
-import { ValidationWorkflowService } from "./validation-workflow.service";
+import { Validation, ValidationError, ValidationWorkflowService } from
"./validation-workflow.service";
import {
mockPoint,
mockResultPredicate,
@@ -301,3 +301,93 @@ describe("ValidationWorkflowService", () => {
subscription.unsubscribe();
});
});
+
+describe("ValidationWorkflowService.combineValidation", () => {
+ const invalid = (messages: Record<string, string>): Validation => ({
isValid: false, messages });
+
+ it("should be valid when given no validations at all", () => {
+ const combined = ValidationWorkflowService.combineValidation();
+
+ expect(combined.isValid).toBe(true);
+ // The valid branch returns { isValid } only, so consumers reading
`messages`
+ // off a valid result get undefined rather than an empty object.
+ expect((combined as ValidationError).messages).toBeUndefined();
+ });
+
+ it("should be valid, with no messages, when every validation is valid", ()
=> {
+ const combined = ValidationWorkflowService.combineValidation({ isValid:
true }, { isValid: true });
+
+ expect(combined).toEqual({ isValid: true });
+ });
+
+ it("should be invalid and carry the messages when a single validation is
invalid", () => {
+ const combined = ValidationWorkflowService.combineValidation(
+ { isValid: true },
+ invalid({ jsonSchema: "property 'x' is required" })
+ );
+
+ expect(combined).toEqual({ isValid: false, messages: { jsonSchema:
"property 'x' is required" } });
+ });
+
+ it("should stay invalid regardless of where the invalid validation sits", ()
=> {
+ const failure = invalid({ connection: "operator has no input" });
+
+ expect(ValidationWorkflowService.combineValidation(failure, { isValid:
true }).isValid).toBe(false);
+ expect(ValidationWorkflowService.combineValidation({ isValid: true },
failure).isValid).toBe(false);
+ });
+
+ it("should merge the messages of several invalid validations", () => {
+ const combined = ValidationWorkflowService.combineValidation(
+ invalid({ jsonSchema: "property 'x' is required" }),
+ { isValid: true },
+ invalid({ connection: "operator has no input" })
+ );
+
+ expect(combined).toEqual({
+ isValid: false,
+ messages: {
+ jsonSchema: "property 'x' is required",
+ connection: "operator has no input",
+ },
+ });
+ });
+
+ it("should let a later message win when two invalid validations share a
key", () => {
+ const combined = ValidationWorkflowService.combineValidation(
+ invalid({ connection: "first" }),
+ invalid({ connection: "second" })
+ );
+
+ expect(combined).toEqual({ isValid: false, messages: { connection:
"second" } });
+ });
+
+ it("should ignore messages attached to a validation that reports itself
valid", () => {
+ // The Validation union gives the valid arm no `messages`, but the merge is
+ // guarded on isValid rather than on the key being absent, so a stray field
+ // is dropped instead of leaking into the combined result.
+ const validWithStrayMessages = { isValid: true, messages: { ignored: "not
a real error" } } as Validation;
+
+ const combined = ValidationWorkflowService.combineValidation(
+ validWithStrayMessages,
+ invalid({ connection: "operator has no input" })
+ );
+
+ expect(combined).toEqual({ isValid: false, messages: { connection:
"operator has no input" } });
+ });
+
+ it("should report invalid with an empty message map when the failing
validation has none", () => {
+ const combined = ValidationWorkflowService.combineValidation(invalid({}));
+
+ expect(combined).toEqual({ isValid: false, messages: {} });
+ });
+
+ it("should not mutate the validations it was given", () => {
+ const first = invalid({ jsonSchema: "property 'x' is required" });
+ const second = invalid({ connection: "operator has no input" });
+
+ ValidationWorkflowService.combineValidation(first, second);
+
+ expect(first).toEqual({ isValid: false, messages: { jsonSchema: "property
'x' is required" } });
+ expect(second).toEqual({ isValid: false, messages: { connection: "operator
has no input" } });
+ });
+});
