Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chore: Improve workflow validation #1344

Merged
merged 4 commits into from
Oct 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 22 additions & 7 deletions packages/cli/src/__tests__/e2e/run.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -208,10 +208,10 @@ describe("e2e tests for run command", () => {

const output = parseOutput(stdout);
expect(output.filter((o => o.status === "SUCCEED"))).toHaveLength(output.length);
expect(output.filter((o => o.validation?.startsWith("SUCCEED")))).toHaveLength(output.length);
expect(output.filter((o => o.validation?.status.startsWith("SUCCEED")))).toHaveLength(output.length);
});

it("Should print error on stderr if validation fails", async () => {
it("Should print error on validation if it fails", async () => {
const testCaseDir = getTestCaseDir(5);
const args = getCmdArgs(testCaseDir);
const { exitCode, stdout, stderr } = await runCLI({
Expand All @@ -225,8 +225,8 @@ describe("e2e tests for run command", () => {
const output = parseOutput(stdout);

expect(output[0].status).toBe("SUCCEED");
expect(output[0].validation).toBe("FAILED");
expect(output[0].error).toBeTruthy();
expect(output[0].validation.status).toBe("FAILED");
expect(output[0].validation.error).toBeTruthy();

expect(stderr).toBeDefined();
expect(stderr).not.toBe("");
Expand Down Expand Up @@ -263,7 +263,7 @@ describe("e2e tests for run command", () => {

const output = parseOutput(stdout);
expect(output[0].status).toBe("SUCCEED");
expect(output[0].validation).toBe("SUCCEED");
expect(output[0].validation.status).toBe("SUCCEED");
expect(output[0].error).toBeFalsy();
});

Expand All @@ -282,7 +282,7 @@ describe("e2e tests for run command", () => {

const output = parseOutput(stdout);
expect(output.filter((o => o.status === "SUCCEED"))).toHaveLength(output.length);
expect(output.filter((o => o.validation?.startsWith("SUCCEED")))).toHaveLength(output.length);
expect(output.filter((o => o.validation.status.startsWith("SUCCEED")))).toHaveLength(output.length);
});

it("Should print error on stderr if job is named 'data' or 'error'", async () => {
Expand Down Expand Up @@ -316,7 +316,22 @@ describe("e2e tests for run command", () => {
const output = parseOutput(stdout);
expect(output[0].id).toBe("case2.0");
expect(output[0].status).toBe("SUCCEED");
expect(output[0].validation).toBe("SUCCEED");
expect(output[0].validation.status).toBe("SUCCEED");
expect(output[0].error).toBeFalsy();
});

it("Should validate expected error", async () => {
const testCaseDir = getTestCaseDir(11);
const { exitCode, stdout } = await runCLI({
args: ["run"],
cwd: testCaseDir,
cli: polywrapCli,
});

expect(exitCode).toEqual(0);

const output = parseOutput(stdout);
expect(output[0].status).toBe("FAILED");
expect(output[0].validation.status).toBe("SUCCEED");
})
});
77 changes: 45 additions & 32 deletions packages/cli/src/__tests__/e2e/utils.ts
Original file line number Diff line number Diff line change
@@ -1,53 +1,66 @@
import {Status, ValidationResult, WorkflowOutput} from "../../lib";

export const clearStyle = (styled: string) => {
return styled.replace(
/[\u001b\u009b][[()#;?]*(?:[0-9]{1,4}(?:;[0-9]{0,4})*)?[0-9A-ORZcf-nqry=><]/g,
""
);
};

export interface WorkflowOutput {
id: string;
data?: unknown;
error?: unknown;
validation?: string;
status: string;
}

export const parseOutput = (
outputs: string
): Array<WorkflowOutput> => {
const outputsArr = outputs.split(/-{35}[\t \n]+-{35}/);
return outputsArr.map((output) => {
output = output.replace(/-{35}/, "");
const idIdx = output.indexOf("ID: ");
const statusIdx = output.indexOf("Status: ");
const dataIdx = output.indexOf("Data: ");
const validationIdx = output.indexOf("Validation: ");
const errIdx = output.indexOf("Error: ");
const id = "ID: ";
const status = "Job status: ";
const data = "Data: ";
const validation = "Validation status: ";
const validationError = "Validation error: ";
const error = "Error: ";
const idIndex = output.indexOf(id);
const statusIndex = output.indexOf(status);
const dataIndex = output.indexOf(data);
const validationIndex = output.indexOf(validation);
const validationErrorIndex = output.indexOf(validationError);
const errorIndex = output.indexOf(error);

const result: Partial<WorkflowOutput> = {};

result.id = output.substring(idIdx + 3, statusIdx - 1).replace(/[\s-]+/g, "")
result.status = output.substring(statusIdx + 8, dataIdx - 1);
result.validation =
validationIdx !== -1
? errIdx !== -1
? output.substring(validationIdx + 12, errIdx).replace(/[\s-]+/g, "")
: output.substring(validationIdx + 12).replace(/[\s-]+/g, "")
: undefined;

if (dataIdx !== -1) {
const data =
validationIdx !== -1
? output.substring(dataIdx + 6, validationIdx - 1)
: output.substring(dataIdx + 6);
result.data = JSON.parse(data);
result.id = output.substring(idIndex + id.length, statusIndex - 1).replace(/[\s-]+/g, "")

const statusEndIndex = dataIndex !== -1 ? dataIndex - 1 : errorIndex - 1
result.status = output.substring(statusIndex + status.length, statusEndIndex) as Status;

const validationStatus = validationIndex !== -1 ?
validationErrorIndex !== -1 ?
output.substring(validationIndex + validation.length, validationErrorIndex).replace(/[\s-]+/g, "") :
output.substring(validationIndex + validation.length).replace(/[\s-]+/g, "")
: undefined

const validationErrorMessage = validationErrorIndex !== -1 ?
output.substring(validationErrorIndex + validationError.length).replace(/[\s-]+/g, "")
: undefined
if (validationStatus) {
result.validation = { status: validationStatus } as ValidationResult
if (validationErrorMessage) {
result.validation.error = validationErrorMessage
}
}

if (dataIndex !== -1) {
const outputData =
validationIndex !== -1
? output.substring(dataIndex + data.length, validationIndex - 1)
: output.substring(dataIndex + data.length);
result.data = JSON.parse(outputData);
}
if (errIdx !== -1) {
result.error =
errIdx !== -1
? output.substring(errIdx + 6).replace(/[\s-]+/g, "")
if (errorIndex !== -1) {
const message = errorIndex !== -1
? output.substring(errorIndex + error.length).replace(/[\s-]+/g, "")
: undefined;
result.error = new Error(message)
}
return result as WorkflowOutput;
});
Expand Down
14 changes: 7 additions & 7 deletions packages/cli/src/__tests__/unit/jobrunner-test-cases.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { MaybeAsync } from "@polywrap/core-js";
import { JobResult, JobStatus } from "../../lib";
import { JobResult, Status } from "../../lib";
import { PolywrapWorkflow } from "@polywrap/polywrap-manifest-types-js";
import { GetPathToTestWrappers } from "@polywrap/test-cases";
import path from "path";
Expand Down Expand Up @@ -40,7 +40,7 @@ export const testCases: WorkflowTestCase[] = [
onExecution: (id: string, jobResult: JobResult) => {
switch (id) {
case "ops.0": {
expect(jobResult.status).toBe(JobStatus.SUCCEED);
expect(jobResult.status).toBe(Status.SUCCEED);
expect(jobResult.error).toBeFalsy();
expect(jobResult.data).toBe(20);
break;
Expand Down Expand Up @@ -93,13 +93,13 @@ export const testCases: WorkflowTestCase[] = [
onExecution: (id: string, jobResult: JobResult) => {
switch (id) {
case "ops.0": {
expect(jobResult.status).toBe(JobStatus.SUCCEED);
expect(jobResult.status).toBe(Status.SUCCEED);
expect(jobResult.error).toBeFalsy();
expect(jobResult.data).toBe(20);
break;
}
case "ops.1": {
expect(jobResult.status).toBe(JobStatus.SUCCEED);
expect(jobResult.status).toBe(Status.SUCCEED);
expect(jobResult.error).toBeFalsy();
expect(jobResult.data).toBe(15);
break;
Expand Down Expand Up @@ -171,19 +171,19 @@ export const testCases: WorkflowTestCase[] = [
onExecution: (id: string, jobResult: JobResult) => {
switch (id) {
case "ops.0": {
expect(jobResult.status).toBe(JobStatus.SUCCEED);
expect(jobResult.status).toBe(Status.SUCCEED);
expect(jobResult.error).toBeFalsy();
expect(jobResult.data).toBe(20);
break;
}
case "ops.1": {
expect(jobResult.status).toBe(JobStatus.SUCCEED);
expect(jobResult.status).toBe(Status.SUCCEED);
expect(jobResult.error).toBeFalsy();
expect(jobResult.data).toBe(15);
break;
}
case "ops.subOps.0": {
expect(jobResult.status).toBe(JobStatus.SUCCEED);
expect(jobResult.status).toBe(Status.SUCCEED);
expect(jobResult.error).toBeFalsy();
expect(jobResult.data).toBe(5);
break;
Expand Down
35 changes: 20 additions & 15 deletions packages/cli/src/commands/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,14 @@ import {
intlMsg,
JobResult,
JobRunner,
JobStatus,
Status,
loadValidationScript,
loadWorkflowManifest,
parseClientConfigOption,
parseWorkflowOutputFilePathOption,
printJobOutput,
validateJobNames,
validateOutput,
ValidationResult,
WorkflowOutput,
defaultWorkflowManifest,
parseManifestFileOption,
Expand Down Expand Up @@ -94,22 +93,24 @@ const _run = async (options: WorkflowCommandOptions) => {

const onExecution = (id: string, jobResult: JobResult) => {
const { data, error, status } = jobResult;
const output: WorkflowOutput = {
id,
status,
data,
error,
validation: {
status: Status.SKIPPED,
},
};

if (error !== undefined) {
process.exit(1);
}

const output: WorkflowOutput = { id, status, data, error };
workflowOutput.push(output);

let validation: ValidationResult | undefined = undefined;
if (status === JobStatus.SUCCEED && validationScript) {
validation = validateOutput(output, validationScript, logger);
if (validationScript) {
validateOutput(output, validationScript, logger);
}

if (!quiet) {
printJobOutput(output, validation);
printJobOutput(output);
}
workflowOutput.push(output);
};

const jobRunner = new JobRunner(client, onExecution);
Expand All @@ -118,13 +119,17 @@ const _run = async (options: WorkflowCommandOptions) => {
if (outputFile) {
const outputFileExt = path.extname(outputFile).substring(1);
if (!outputFileExt) throw new Error("Require output file extension");
const printableOutput = workflowOutput.map((o) => ({
...o,
error: o.error?.message,
}));
switch (outputFileExt) {
case "yaml":
case "yml":
fs.writeFileSync(outputFile, yaml.stringify(workflowOutput, null, 2));
fs.writeFileSync(outputFile, yaml.stringify(printableOutput, null, 2));
break;
case "json":
fs.writeFileSync(outputFile, JSON.stringify(workflowOutput, null, 2));
fs.writeFileSync(outputFile, JSON.stringify(printableOutput, null, 2));
break;
default:
throw new Error(
Expand Down
13 changes: 8 additions & 5 deletions packages/cli/src/lib/helpers/workflow-validator.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { runCommandSync } from "../system";
import { Logger } from "../logging";
import { intlMsg } from "../intl";
import { JobStatus, ValidationResult, WorkflowOutput } from "../workflow";
import { Status, WorkflowOutput } from "../workflow";

import path from "path";
import fs from "fs";
Expand All @@ -18,7 +18,7 @@ export function validateOutput(
output: WorkflowOutput,
validateScriptPath: string,
logger: Logger
): ValidationResult {
): void {
if (!cueExists(logger)) {
console.warn(intlMsg.commands_run_error_cueDoesNotExist());
}
Expand All @@ -32,7 +32,10 @@ export function validateOutput(
const selector = `${jobId}.\\$${stepId}`;
const jsonOutput = `${TMPDIR}/${id}.json`;

fs.writeFileSync(jsonOutput, JSON.stringify({ data, error }, null, 2));
fs.writeFileSync(
jsonOutput,
JSON.stringify({ data, error: error?.message }, null, 2)
);

const { stderr } = runCommandSync(
`cue vet -d ${selector} ${validateScriptPath} ${jsonOutput}`,
Expand All @@ -44,9 +47,9 @@ export function validateOutput(
}

if (!stderr) {
return { status: JobStatus.SUCCEED };
output.validation = { status: Status.SUCCEED };
} else {
process.exitCode = 1;
return { status: JobStatus.FAILED, stderr: stderr.stderr };
output.validation = { status: Status.FAILED, error: stderr.stderr };
}
}
14 changes: 7 additions & 7 deletions packages/cli/src/lib/workflow/JobRunner.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { JobResult, JobStatus, Step } from "./types";
import { JobResult, Status, Step } from "./types";

import { PolywrapClient } from "@polywrap/client-js";
import { MaybeAsync } from "@polywrap/core-js";
Expand Down Expand Up @@ -91,20 +91,20 @@ export class JobRunner {
const accessors: string[] = reference
.substring(dataOrErrorIdx + 1)
.split(".");
if (refJobResult.status === JobStatus.SKIPPED) {
if (refJobResult.status === Status.SKIPPED) {
throw new Error(
`Tried to resolve reference to skipped job ${referenceId} for step ${absJobId}.${stepId}`
);
} else if (
accessors[0] === "data" &&
refJobResult.status === JobStatus.FAILED
refJobResult.status === Status.FAILED
) {
throw new Error(
`Tried to resolve data of failed job ${referenceId} for step ${absJobId}.${stepId}`
);
} else if (
accessors[0] === "error" &&
refJobResult.status === JobStatus.SUCCEED
refJobResult.status === Status.SUCCEED
) {
throw new Error(
`Tried to resolve error message of successful job ${referenceId} for step ${absJobId}.${stepId}`
Expand Down Expand Up @@ -169,7 +169,7 @@ export class JobRunner {
} catch (e) {
return {
error: e,
status: JobStatus.SKIPPED,
status: Status.SKIPPED,
};
}
}
Expand All @@ -192,9 +192,9 @@ export class JobRunner {
});

if (!invokeResult.ok) {
return { error: invokeResult.error, status: JobStatus.FAILED };
return { error: invokeResult.error, status: Status.FAILED };
} else {
return { data: invokeResult.value, status: JobStatus.SUCCEED };
return { data: invokeResult.value, status: Status.SUCCEED };
}
}

Expand Down
7 changes: 1 addition & 6 deletions packages/cli/src/lib/workflow/index.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@
export * from "./JobRunner";
export {
JobStatus,
JobResult,
ValidationResult,
WorkflowOutput,
} from "./types";
export { Status, JobResult, ValidationResult, WorkflowOutput } from "./types";
export * from "./util";
Loading