Skip to content

Commit

Permalink
Validate required inputs in tests documents + tests
Browse files Browse the repository at this point in the history
  • Loading branch information
davelopez committed Jun 20, 2024
1 parent 6ab97c0 commit 815b7d9
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 7 deletions.
10 changes: 10 additions & 0 deletions server/packages/server-common/tests/testHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export const FAKE_DATASET_INPUT: WorkflowInput = {
name: "My fake dataset",
doc: "This is a simple dataset",
type: "data",
optional: true,
};

export const EXPECTED_WORKFLOW_INPUTS: WorkflowInput[] = [
Expand All @@ -65,11 +66,13 @@ export const EXPECTED_WORKFLOW_INPUTS: WorkflowInput[] = [
name: "Input dataset: fake",
doc: "This is a simple dataset with a colon in the name",
type: "File",
optional: true,
},
{
name: "My fake collection",
doc: "This is a collection",
type: "collection",
optional: true,
},
{
name: "My fake string",
Expand All @@ -84,6 +87,13 @@ export const EXPECTED_WORKFLOW_INPUTS: WorkflowInput[] = [
type: "int",
optional: false,
},
{
name: "My fake boolean",
doc: "This is a required boolean parameter with a default value",
type: "boolean",
default: true,
optional: false,
},
];

export const EXPECTED_WORKFLOW_OUTPUTS: WorkflowOutput[] = [
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { ValidationRule, WorkflowTestsDocument } from "@gxwf/server-common/src/languageTypes";
import { PropertyASTNode } from "@gxwf/server-common/src/ast/types";
import { ValidationRule, WorkflowInput, WorkflowTestsDocument } from "@gxwf/server-common/src/languageTypes";
import { isCompatibleType } from "@gxwf/server-common/src/utils";
import { Diagnostic, DiagnosticSeverity } from "vscode-languageserver-types";

Expand All @@ -11,13 +12,32 @@ export class WorkflowInputsValidationRule implements ValidationRule {
public async validate(documentContext: WorkflowTestsDocument): Promise<Diagnostic[]> {
const diagnostics: Diagnostic[] = [];
const workflowInputs = await documentContext.getWorkflowInputs();
const documentInputNodes =
documentContext.nodeManager.getAllPropertyNodesByName("job")[0]?.valueNode?.children ?? [];
const jobNodes = documentContext.nodeManager.getAllPropertyNodesByName("job");
for (const jobNode of jobNodes) {
const jobDiagnostics = await this.validateInputsInJobNode(jobNode, workflowInputs, documentContext);
diagnostics.push(...jobDiagnostics);
}
return Promise.resolve(diagnostics);
}

private async validateInputsInJobNode(
jobNode: PropertyASTNode,
workflowInputs: WorkflowInput[],
documentContext: WorkflowTestsDocument
): Promise<Diagnostic[]> {
const diagnostics: Diagnostic[] = [];
if (!jobNode) {
// Skip validation if the job node is not present. This will be caught by schema validation.
return Promise.resolve(diagnostics);
}
const documentInputNodes = jobNode?.valueNode?.children ?? [];
const definedInputsInTestDocument = new Set<string>();
for (const inputNode of documentInputNodes) {
if (inputNode.type !== "property") {
continue;
}
const inputName = inputNode.keyNode.value as string;
definedInputsInTestDocument.add(inputName);
const input = workflowInputs.find((i) => i.name === inputName);
if (!input) {
const range = documentContext.nodeManager.getNodeRange(inputNode);
Expand All @@ -35,6 +55,16 @@ export class WorkflowInputsValidationRule implements ValidationRule {
}
}
}
const requiredWorkflowInputs = workflowInputs
.filter((i) => !i.optional && i.default === undefined)
.map((i) => i.name);
const range = documentContext.nodeManager.getNodeRange(jobNode);
for (const requiredInput of requiredWorkflowInputs) {
if (!definedInputsInTestDocument.has(requiredInput)) {
const message = `Input "${requiredInput}" is required but no value or default was provided.`;
diagnostics.push(Diagnostic.create(range, message, DiagnosticSeverity.Error));
}
}
return Promise.resolve(diagnostics);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ describe("Workflow Tests Validation Rules", () => {
- doc: The docs
job:
My fake dataset: data/input.txt
My fake number: 5
`;

const diagnostics = await validate(testDocumentContents);
Expand All @@ -80,6 +81,7 @@ describe("Workflow Tests Validation Rules", () => {
job:
My fake dataset:
class: File
My fake number: 5
`;

const diagnostics = await validate(testDocumentContents);
Expand All @@ -92,20 +94,22 @@ describe("Workflow Tests Validation Rules", () => {
const testDocumentContents = `
- doc: The docs
job:
Missing input: data/input.txt
My fake number: 5
Unknown input: data/input.txt
`;

const diagnostics = await validate(testDocumentContents);

expect(diagnostics.length).toBe(1);
expect(diagnostics[0].message).toBe('Input "Missing input" is not defined in the associated workflow.');
expect(diagnostics[0].message).toBe('Input "Unknown input" is not defined in the associated workflow.');
expect(diagnostics[0].severity).toBe(DiagnosticSeverity.Error);
});

it("should error when an input has an invalid type (inline)", async () => {
const testDocumentContents = `
- doc: The docs
job:
My fake number: 5
My fake string: 5
`;

Expand All @@ -122,18 +126,48 @@ describe("Workflow Tests Validation Rules", () => {
const testDocumentContents = `
- doc: The docs
job:
My fake number:
My fake number: 5
My fake string:
class: string
`;

const diagnostics = await validate(testDocumentContents);

expect(diagnostics.length).toBe(1);
expect(diagnostics[0].message).toBe(
'Input "My fake number" has an invalid type. Expected "int" but found "object".'
'Input "My fake string" has an invalid type. Expected "string" but found "object".'
);
expect(diagnostics[0].severity).toBe(DiagnosticSeverity.Error);
});

it("should error when an input is required but not defined", async () => {
const testDocumentContents = `
- doc: The docs
job:
`;

const diagnostics = await validate(testDocumentContents);

expect(diagnostics.length).toBe(1);
expect(diagnostics[0].message).toBe('Input "My fake number" is required but no value or default was provided.');
expect(diagnostics[0].severity).toBe(DiagnosticSeverity.Error);
});

it("should error when an input is required in multiple tests but not defined in one of them", async () => {
const testDocumentContents = `
- doc: The docs
job:
My fake number: 5
- doc: The docs
job:
`;

const diagnostics = await validate(testDocumentContents);

expect(diagnostics.length).toBe(1);
expect(diagnostics[0].message).toBe('Input "My fake number" is required but no value or default was provided.');
expect(diagnostics[0].severity).toBe(DiagnosticSeverity.Error);
});
});

describe("WorkflowOutputsValidationRule", () => {
Expand Down

0 comments on commit 815b7d9

Please sign in to comment.