Improve cron schedule validation and diagnostics (#224)
This commit is contained in:
@@ -110,11 +110,8 @@ jobs:
|
||||
`;
|
||||
const result = await hover(...getPositionFromCursor(input));
|
||||
expect(result).not.toBeUndefined();
|
||||
expect(result?.contents).toEqual(
|
||||
"Runs at 0 and 30 minutes past the hour, at 00:00 and 12:00\n\n" +
|
||||
"Actions schedules run at most every 5 minutes. " +
|
||||
"[Learn more](https://docs.github.com/actions/using-workflows/workflow-syntax-for-github-actions#onschedule)"
|
||||
);
|
||||
// Cron description is now shown via diagnostics, not hover
|
||||
expect(result?.contents).toEqual("");
|
||||
});
|
||||
|
||||
it("on a cron mapping key", async () => {
|
||||
|
||||
@@ -2,11 +2,9 @@ import {data, DescriptionDictionary, Parser} from "@actions/expressions";
|
||||
import {FunctionDefinition, FunctionInfo} from "@actions/expressions/funcs/info";
|
||||
import {Lexer} from "@actions/expressions/lexer";
|
||||
import {ErrorPolicy} from "@actions/workflow-parser/model/convert";
|
||||
import {getCronDescription} from "@actions/workflow-parser/model/converter/cron";
|
||||
import {splitAllowedContext} from "@actions/workflow-parser/templates/allowed-context";
|
||||
import {StringToken} from "@actions/workflow-parser/templates/tokens/string-token";
|
||||
import {TemplateToken} from "@actions/workflow-parser/templates/tokens/template-token";
|
||||
import {isBasicExpression, isString} from "@actions/workflow-parser/templates/tokens/type-guards";
|
||||
import {isBasicExpression} from "@actions/workflow-parser/templates/tokens/type-guards";
|
||||
import {File} from "@actions/workflow-parser/workflows/file";
|
||||
import {FileProvider} from "@actions/workflow-parser/workflows/file-provider";
|
||||
import {Position, TextDocument} from "vscode-languageserver-textdocument";
|
||||
@@ -23,7 +21,7 @@ import {ExpressionPos, mapToExpressionPos} from "./expression-hover/expression-p
|
||||
import {HoverVisitor} from "./expression-hover/visitor";
|
||||
import {info} from "./log";
|
||||
import {isPotentiallyExpression} from "./utils/expression-detection";
|
||||
import {findToken, TokenResult} from "./utils/find-token";
|
||||
import {findToken} from "./utils/find-token";
|
||||
import {mapRange} from "./utils/range";
|
||||
import {fetchOrConvertWorkflowTemplate, fetchOrParseWorkflow} from "./utils/workflow-cache";
|
||||
|
||||
@@ -89,17 +87,6 @@ export async function hover(document: TextDocument, position: Position, config?:
|
||||
|
||||
info(`Calculating hover for token with definition ${token.definition.key}`);
|
||||
|
||||
if (tokenResult.parent && isCronMappingValue(tokenResult)) {
|
||||
const tokenValue = (token as StringToken).value;
|
||||
const description = getCronDescription(tokenValue);
|
||||
if (description) {
|
||||
return {
|
||||
contents: description,
|
||||
range: mapRange(token.range)
|
||||
} satisfies Hover;
|
||||
}
|
||||
}
|
||||
|
||||
if (tokenResult.parent && isReusableWorkflowJobInput(tokenResult)) {
|
||||
let description = getReusableWorkflowInputDescription(workflowContext, tokenResult);
|
||||
description = appendContext(description, token.definitionInfo?.allowedContext);
|
||||
@@ -156,15 +143,6 @@ async function getDescription(
|
||||
return description || defaultDescription;
|
||||
}
|
||||
|
||||
function isCronMappingValue(tokenResult: TokenResult): boolean {
|
||||
return (
|
||||
tokenResult.parent?.definition?.key === "cron-mapping" &&
|
||||
!!tokenResult.token &&
|
||||
isString(tokenResult.token) &&
|
||||
tokenResult.token.value !== "cron"
|
||||
);
|
||||
}
|
||||
|
||||
function expressionHover(
|
||||
exprPos: ExpressionPos,
|
||||
context: DescriptionDictionary,
|
||||
|
||||
@@ -181,7 +181,7 @@ jobs:
|
||||
|
||||
expect(result.length).toBe(1);
|
||||
expect(result[0]).toEqual({
|
||||
message: "Invalid cron string",
|
||||
message: "Invalid cron expression. Expected format: '* * * * *' (minute hour day month weekday)",
|
||||
range: {
|
||||
end: {
|
||||
character: 21,
|
||||
@@ -195,6 +195,95 @@ jobs:
|
||||
} as Diagnostic);
|
||||
});
|
||||
|
||||
it("cron with interval less than 5 minutes shows warning", async () => {
|
||||
const result = await validate(
|
||||
createDocument(
|
||||
"wf.yaml",
|
||||
`on:
|
||||
schedule:
|
||||
- cron: '*/1 * * * *'
|
||||
jobs:
|
||||
build:
|
||||
runs-on: ubuntu-latest`
|
||||
),
|
||||
{valueProviderConfig: defaultValueProviders}
|
||||
);
|
||||
|
||||
expect(result.length).toBe(1);
|
||||
expect(result[0]).toEqual({
|
||||
message: "Runs every minute. Note: Actions schedules run at most every 5 minutes.",
|
||||
severity: DiagnosticSeverity.Warning,
|
||||
code: "on-schedule",
|
||||
codeDescription: {
|
||||
href: "https://docs.github.com/actions/using-workflows/workflow-syntax-for-github-actions#onschedule"
|
||||
},
|
||||
range: {
|
||||
end: {
|
||||
character: 25,
|
||||
line: 2
|
||||
},
|
||||
start: {
|
||||
character: 12,
|
||||
line: 2
|
||||
}
|
||||
}
|
||||
} as Diagnostic);
|
||||
});
|
||||
|
||||
it("cron with interval of 5 minutes or more shows info", async () => {
|
||||
const result = await validate(
|
||||
createDocument(
|
||||
"wf.yaml",
|
||||
`on:
|
||||
schedule:
|
||||
- cron: '*/5 * * * *'
|
||||
jobs:
|
||||
build:
|
||||
runs-on: ubuntu-latest`
|
||||
),
|
||||
{valueProviderConfig: defaultValueProviders}
|
||||
);
|
||||
|
||||
expect(result.length).toBe(1);
|
||||
expect(result[0]).toEqual({
|
||||
message: "Runs every 5 minutes",
|
||||
severity: DiagnosticSeverity.Information,
|
||||
code: "on-schedule",
|
||||
codeDescription: {
|
||||
href: "https://docs.github.com/actions/using-workflows/workflow-syntax-for-github-actions#onschedule"
|
||||
},
|
||||
range: {
|
||||
end: {
|
||||
character: 25,
|
||||
line: 2
|
||||
},
|
||||
start: {
|
||||
character: 12,
|
||||
line: 2
|
||||
}
|
||||
}
|
||||
} as Diagnostic);
|
||||
});
|
||||
|
||||
it("cron with comma-separated minutes less than 5 apart shows warning", async () => {
|
||||
const result = await validate(
|
||||
createDocument(
|
||||
"wf.yaml",
|
||||
`on:
|
||||
schedule:
|
||||
- cron: '0,2 * * * *'
|
||||
jobs:
|
||||
build:
|
||||
runs-on: ubuntu-latest`
|
||||
),
|
||||
{valueProviderConfig: defaultValueProviders}
|
||||
);
|
||||
|
||||
expect(result.length).toBe(1);
|
||||
expect(result[0]?.severity).toBe(DiagnosticSeverity.Warning);
|
||||
expect(result[0]?.message).toContain("Note: Actions schedules run at most every 5 minutes.");
|
||||
});
|
||||
|
||||
it("invalid YAML", async () => {
|
||||
// This YAML has some mismatched single-quotes, which causes the string to be terminated early
|
||||
// within the fromJSON() expression.
|
||||
|
||||
@@ -2,6 +2,7 @@ import {Lexer, Parser, data} from "@actions/expressions";
|
||||
import {Expr, FunctionCall, Literal, Logical} from "@actions/expressions/ast";
|
||||
import {ParseWorkflowResult, WorkflowTemplate, isBasicExpression, isString} from "@actions/workflow-parser";
|
||||
import {ErrorPolicy} from "@actions/workflow-parser/model/convert";
|
||||
import {getCronDescription, hasCronIntervalLessThan5Minutes} from "@actions/workflow-parser/model/converter/cron";
|
||||
import {ensureStatusFunction} from "@actions/workflow-parser/model/converter/if-condition";
|
||||
import {splitAllowedContext} from "@actions/workflow-parser/templates/allowed-context";
|
||||
import {BasicExpressionToken} from "@actions/workflow-parser/templates/tokens/basic-expression-token";
|
||||
@@ -27,6 +28,9 @@ import {validateAction} from "./validate-action";
|
||||
import {ValueProviderConfig, ValueProviderKind} from "./value-providers/config";
|
||||
import {defaultValueProviders} from "./value-providers/default";
|
||||
|
||||
const CRON_SCHEDULE_DOCS_URL =
|
||||
"https://docs.github.com/actions/using-workflows/workflow-syntax-for-github-actions#onschedule";
|
||||
|
||||
export type ValidationConfig = {
|
||||
valueProviderConfig?: ValueProviderConfig;
|
||||
contextProviderConfig?: ContextProviderConfig;
|
||||
@@ -166,6 +170,11 @@ async function additionalValidations(
|
||||
validateWorkflowUsesFormat(diagnostics, token);
|
||||
}
|
||||
|
||||
// Validate cron expressions - warn if interval is less than 5 minutes
|
||||
if (isString(token) && token.range && validationDefinition?.key === "cron-pattern") {
|
||||
validateCronExpression(diagnostics, token);
|
||||
}
|
||||
|
||||
// Allowed values coming from the schema have already been validated. Only check if
|
||||
// a value provider is defined for a token and if it is, validate the values match.
|
||||
if (token.range && validationDefinition) {
|
||||
@@ -216,6 +225,50 @@ function invalidValue(diagnostics: Diagnostic[], token: StringToken, kind: Value
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Validates cron expressions and provides diagnostics for valid cron schedules.
|
||||
* Shows a warning if the interval is less than 5 minutes (since GitHub Actions
|
||||
* schedules run at most every 5 minutes), otherwise shows an info message.
|
||||
*/
|
||||
function validateCronExpression(diagnostics: Diagnostic[], token: StringToken): void {
|
||||
const cronValue = token.value;
|
||||
|
||||
// Ensure we have a range for diagnostics
|
||||
if (!token.range) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Only check valid cron expressions - invalid ones are already caught by the parser
|
||||
const description = getCronDescription(cronValue);
|
||||
if (!description) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Check if the cron specifies an interval less than 5 minutes
|
||||
if (hasCronIntervalLessThan5Minutes(cronValue)) {
|
||||
diagnostics.push({
|
||||
message: `${description}. Note: Actions schedules run at most every 5 minutes.`,
|
||||
range: mapRange(token.range),
|
||||
severity: DiagnosticSeverity.Warning,
|
||||
code: "on-schedule",
|
||||
codeDescription: {
|
||||
href: CRON_SCHEDULE_DOCS_URL
|
||||
}
|
||||
});
|
||||
} else {
|
||||
// Show info message for valid cron expressions
|
||||
diagnostics.push({
|
||||
message: description,
|
||||
range: mapRange(token.range),
|
||||
severity: DiagnosticSeverity.Information,
|
||||
code: "on-schedule",
|
||||
codeDescription: {
|
||||
href: CRON_SCHEDULE_DOCS_URL
|
||||
}
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Validates the format of a step's `uses` field.
|
||||
*
|
||||
|
||||
Reference in New Issue
Block a user