fix: always provide strategy and matrix contexts with defaults (#235)
Fixes https://github.com/github/vscode-github-actions/issues/113 The strategy and matrix contexts are always available in job steps, even when no strategy block is defined. Changes: - Remove the hasStrategy filter from filterContextNames in default.ts - Return null for matrix when no strategy is defined - Provide default values for strategy properties: - fail-fast: true - job-index: 0 - job-total: 1 - max-parallel: 1 - Use defaults for missing strategy properties even when strategy IS defined - Add comprehensive unit tests for strategy context This eliminates false positive 'Context access might be invalid' warnings when using strategy.* or matrix in jobs without an explicit strategy block.
This commit is contained in:
@@ -861,7 +861,7 @@ jobs:
|
|||||||
});
|
});
|
||||||
|
|
||||||
describe("strategy context", () => {
|
describe("strategy context", () => {
|
||||||
it("strategy is not suggested when outside of a matrix job", async () => {
|
it("strategy is suggested even when no strategy defined", async () => {
|
||||||
const input = `
|
const input = `
|
||||||
on: push
|
on: push
|
||||||
|
|
||||||
@@ -875,7 +875,7 @@ jobs:
|
|||||||
|
|
||||||
const result = await complete(...getPositionFromCursor(input), {contextProviderConfig});
|
const result = await complete(...getPositionFromCursor(input), {contextProviderConfig});
|
||||||
|
|
||||||
expect(result.map(x => x.label)).not.toContain("strategy");
|
expect(result.map(x => x.label)).toContain("strategy");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("strategy is suggested within a matrix job", async () => {
|
it("strategy is suggested within a matrix job", async () => {
|
||||||
@@ -922,7 +922,7 @@ jobs:
|
|||||||
});
|
});
|
||||||
|
|
||||||
describe("matrix context", () => {
|
describe("matrix context", () => {
|
||||||
it("matrix is not suggested when outside of a matrix job", async () => {
|
it("matrix is suggested even when no strategy defined", async () => {
|
||||||
const input = `
|
const input = `
|
||||||
on: push
|
on: push
|
||||||
|
|
||||||
@@ -936,7 +936,7 @@ jobs:
|
|||||||
|
|
||||||
const result = await complete(...getPositionFromCursor(input), {contextProviderConfig});
|
const result = await complete(...getPositionFromCursor(input), {contextProviderConfig});
|
||||||
|
|
||||||
expect(result.map(x => x.label)).not.toContain("strategy");
|
expect(result.map(x => x.label)).toContain("matrix");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("matrix is suggested within a matrix job", async () => {
|
it("matrix is suggested within a matrix job", async () => {
|
||||||
@@ -1123,10 +1123,12 @@ jobs:
|
|||||||
"github",
|
"github",
|
||||||
"inputs",
|
"inputs",
|
||||||
"job",
|
"job",
|
||||||
|
"matrix",
|
||||||
"needs",
|
"needs",
|
||||||
"runner",
|
"runner",
|
||||||
"secrets",
|
"secrets",
|
||||||
"steps",
|
"steps",
|
||||||
|
"strategy",
|
||||||
"vars",
|
"vars",
|
||||||
"contains",
|
"contains",
|
||||||
"endsWith",
|
"endsWith",
|
||||||
|
|||||||
@@ -32,8 +32,9 @@ export async function getContext(
|
|||||||
): Promise<DescriptionDictionary> {
|
): Promise<DescriptionDictionary> {
|
||||||
const context = new DescriptionDictionary();
|
const context = new DescriptionDictionary();
|
||||||
|
|
||||||
const filteredNames = filterContextNames(names, workflowContext);
|
// All context names are valid - strategy and matrix are always available
|
||||||
for (const contextName of filteredNames) {
|
// (with default values when no strategy block is defined)
|
||||||
|
for (const contextName of names) {
|
||||||
let value = getDefaultContext(contextName, workflowContext, mode) || new DescriptionDictionary();
|
let value = getDefaultContext(contextName, workflowContext, mode) || new DescriptionDictionary();
|
||||||
if (value.kind === Kind.Null) {
|
if (value.kind === Kind.Null) {
|
||||||
context.add(contextName, value);
|
context.add(contextName, value);
|
||||||
@@ -111,18 +112,3 @@ function objectToDictionary(object: {[key: string]: string}): DescriptionDiction
|
|||||||
|
|
||||||
return dictionary;
|
return dictionary;
|
||||||
}
|
}
|
||||||
|
|
||||||
function filterContextNames(contextNames: string[], workflowContext: WorkflowContext): string[] {
|
|
||||||
return contextNames.filter(name => {
|
|
||||||
switch (name) {
|
|
||||||
case "matrix":
|
|
||||||
case "strategy":
|
|
||||||
return hasStrategy(workflowContext);
|
|
||||||
}
|
|
||||||
return true;
|
|
||||||
});
|
|
||||||
}
|
|
||||||
|
|
||||||
function hasStrategy(workflowContext: WorkflowContext): boolean {
|
|
||||||
return workflowContext.job?.strategy !== undefined || workflowContext.reusableWorkflowJob?.strategy !== undefined;
|
|
||||||
}
|
|
||||||
|
|||||||
@@ -64,7 +64,7 @@ describe("matrix context", () => {
|
|||||||
expect(workflowContext.job).toBeUndefined();
|
expect(workflowContext.job).toBeUndefined();
|
||||||
|
|
||||||
const context = getMatrixContext(workflowContext, Mode.Validation);
|
const context = getMatrixContext(workflowContext, Mode.Validation);
|
||||||
expect(context).toEqual(new DescriptionDictionary());
|
expect(context).toEqual(new data.Null());
|
||||||
});
|
});
|
||||||
|
|
||||||
it("strategy not defined", () => {
|
it("strategy not defined", () => {
|
||||||
@@ -73,7 +73,7 @@ describe("matrix context", () => {
|
|||||||
expect(workflowContext.job!.strategy).toBeUndefined();
|
expect(workflowContext.job!.strategy).toBeUndefined();
|
||||||
|
|
||||||
const context = getMatrixContext(workflowContext, Mode.Validation);
|
const context = getMatrixContext(workflowContext, Mode.Validation);
|
||||||
expect(context).toEqual(new DescriptionDictionary());
|
expect(context).toEqual(new data.Null());
|
||||||
});
|
});
|
||||||
|
|
||||||
it("strategy is not a mapping token", () => {
|
it("strategy is not a mapping token", () => {
|
||||||
@@ -81,7 +81,7 @@ describe("matrix context", () => {
|
|||||||
expect(workflowContext.job!.strategy).toBeDefined();
|
expect(workflowContext.job!.strategy).toBeDefined();
|
||||||
|
|
||||||
const context = getMatrixContext(workflowContext, Mode.Validation);
|
const context = getMatrixContext(workflowContext, Mode.Validation);
|
||||||
expect(context).toEqual(new DescriptionDictionary());
|
expect(context).toEqual(new data.Null());
|
||||||
});
|
});
|
||||||
|
|
||||||
it("matrix is not defined", () => {
|
it("matrix is not defined", () => {
|
||||||
|
|||||||
@@ -10,7 +10,8 @@ export function getMatrixContext(workflowContext: WorkflowContext, mode: Mode):
|
|||||||
// https://docs.github.com/en/actions/learn-github-actions/contexts#matrix-context
|
// https://docs.github.com/en/actions/learn-github-actions/contexts#matrix-context
|
||||||
const strategy = workflowContext.job?.strategy ?? workflowContext.reusableWorkflowJob?.strategy;
|
const strategy = workflowContext.job?.strategy ?? workflowContext.reusableWorkflowJob?.strategy;
|
||||||
if (!strategy || !isMapping(strategy)) {
|
if (!strategy || !isMapping(strategy)) {
|
||||||
return new DescriptionDictionary();
|
// No strategy defined - matrix is null at runtime (not empty object)
|
||||||
|
return new data.Null();
|
||||||
}
|
}
|
||||||
|
|
||||||
const matrix = strategy.find("matrix");
|
const matrix = strategy.find("matrix");
|
||||||
|
|||||||
@@ -0,0 +1,126 @@
|
|||||||
|
import {data} from "@actions/expressions";
|
||||||
|
import {Job} from "@actions/workflow-parser/model/workflow-template";
|
||||||
|
import {BooleanToken} from "@actions/workflow-parser/templates/tokens/boolean-token";
|
||||||
|
import {MappingToken} from "@actions/workflow-parser/templates/tokens/mapping-token";
|
||||||
|
import {NumberToken} from "@actions/workflow-parser/templates/tokens/number-token";
|
||||||
|
import {StringToken} from "@actions/workflow-parser/templates/tokens/string-token";
|
||||||
|
import {TemplateToken} from "@actions/workflow-parser/templates/tokens/template-token";
|
||||||
|
import {WorkflowContext} from "../context/workflow-context";
|
||||||
|
import {getStrategyContext} from "./strategy";
|
||||||
|
|
||||||
|
function stringToToken(value: string) {
|
||||||
|
return new StringToken(undefined, undefined, value, undefined);
|
||||||
|
}
|
||||||
|
|
||||||
|
function boolToToken(value: boolean) {
|
||||||
|
return new BooleanToken(undefined, undefined, value, undefined);
|
||||||
|
}
|
||||||
|
|
||||||
|
function numberToToken(value: number) {
|
||||||
|
return new NumberToken(undefined, undefined, value, undefined);
|
||||||
|
}
|
||||||
|
|
||||||
|
function contextFromStrategy(strategy?: TemplateToken) {
|
||||||
|
return {
|
||||||
|
job: {
|
||||||
|
strategy: strategy
|
||||||
|
}
|
||||||
|
} as WorkflowContext;
|
||||||
|
}
|
||||||
|
|
||||||
|
describe("strategy context", () => {
|
||||||
|
describe("no strategy defined", () => {
|
||||||
|
it("returns defaults when job is undefined", () => {
|
||||||
|
const workflowContext = {} as WorkflowContext;
|
||||||
|
|
||||||
|
const context = getStrategyContext(workflowContext);
|
||||||
|
|
||||||
|
expect(context.get("fail-fast")).toEqual(new data.BooleanData(true));
|
||||||
|
expect(context.get("job-index")).toEqual(new data.NumberData(0));
|
||||||
|
expect(context.get("job-total")).toEqual(new data.NumberData(1));
|
||||||
|
expect(context.get("max-parallel")).toEqual(new data.NumberData(1));
|
||||||
|
});
|
||||||
|
|
||||||
|
it("returns defaults when strategy is undefined", () => {
|
||||||
|
const job = {} as Job;
|
||||||
|
const workflowContext = {job} as WorkflowContext;
|
||||||
|
|
||||||
|
const context = getStrategyContext(workflowContext);
|
||||||
|
|
||||||
|
expect(context.get("fail-fast")).toEqual(new data.BooleanData(true));
|
||||||
|
expect(context.get("job-index")).toEqual(new data.NumberData(0));
|
||||||
|
expect(context.get("job-total")).toEqual(new data.NumberData(1));
|
||||||
|
expect(context.get("max-parallel")).toEqual(new data.NumberData(1));
|
||||||
|
});
|
||||||
|
|
||||||
|
it("returns defaults when strategy is not a mapping", () => {
|
||||||
|
const workflowContext = contextFromStrategy(stringToToken("hello"));
|
||||||
|
|
||||||
|
const context = getStrategyContext(workflowContext);
|
||||||
|
|
||||||
|
expect(context.get("fail-fast")).toEqual(new data.BooleanData(true));
|
||||||
|
expect(context.get("job-index")).toEqual(new data.NumberData(0));
|
||||||
|
expect(context.get("job-total")).toEqual(new data.NumberData(1));
|
||||||
|
expect(context.get("max-parallel")).toEqual(new data.NumberData(1));
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("strategy defined with partial properties", () => {
|
||||||
|
it("uses specified fail-fast, defaults for others", () => {
|
||||||
|
const strategy = new MappingToken(undefined, undefined, undefined);
|
||||||
|
strategy.add(stringToToken("fail-fast"), boolToToken(false));
|
||||||
|
const workflowContext = contextFromStrategy(strategy);
|
||||||
|
|
||||||
|
const context = getStrategyContext(workflowContext);
|
||||||
|
|
||||||
|
expect(context.get("fail-fast")).toEqual(new data.BooleanData(false));
|
||||||
|
expect(context.get("job-index")).toEqual(new data.NumberData(0));
|
||||||
|
expect(context.get("job-total")).toEqual(new data.NumberData(1));
|
||||||
|
expect(context.get("max-parallel")).toEqual(new data.NumberData(1));
|
||||||
|
});
|
||||||
|
|
||||||
|
it("uses specified max-parallel, defaults for others", () => {
|
||||||
|
const strategy = new MappingToken(undefined, undefined, undefined);
|
||||||
|
strategy.add(stringToToken("max-parallel"), numberToToken(5));
|
||||||
|
const workflowContext = contextFromStrategy(strategy);
|
||||||
|
|
||||||
|
const context = getStrategyContext(workflowContext);
|
||||||
|
|
||||||
|
expect(context.get("fail-fast")).toEqual(new data.BooleanData(true));
|
||||||
|
expect(context.get("job-index")).toEqual(new data.NumberData(0));
|
||||||
|
expect(context.get("job-total")).toEqual(new data.NumberData(1));
|
||||||
|
expect(context.get("max-parallel")).toEqual(new data.NumberData(5));
|
||||||
|
});
|
||||||
|
|
||||||
|
it("only has matrix defined, all strategy properties use defaults", () => {
|
||||||
|
const strategy = new MappingToken(undefined, undefined, undefined);
|
||||||
|
const matrix = new MappingToken(undefined, undefined, undefined);
|
||||||
|
strategy.add(stringToToken("matrix"), matrix);
|
||||||
|
const workflowContext = contextFromStrategy(strategy);
|
||||||
|
|
||||||
|
const context = getStrategyContext(workflowContext);
|
||||||
|
|
||||||
|
expect(context.get("fail-fast")).toEqual(new data.BooleanData(true));
|
||||||
|
expect(context.get("job-index")).toEqual(new data.NumberData(0));
|
||||||
|
expect(context.get("job-total")).toEqual(new data.NumberData(1));
|
||||||
|
expect(context.get("max-parallel")).toEqual(new data.NumberData(1));
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("strategy with all properties defined", () => {
|
||||||
|
it("uses all specified values", () => {
|
||||||
|
const strategy = new MappingToken(undefined, undefined, undefined);
|
||||||
|
strategy.add(stringToToken("fail-fast"), boolToToken(false));
|
||||||
|
strategy.add(stringToToken("max-parallel"), numberToToken(3));
|
||||||
|
const workflowContext = contextFromStrategy(strategy);
|
||||||
|
|
||||||
|
const context = getStrategyContext(workflowContext);
|
||||||
|
|
||||||
|
expect(context.get("fail-fast")).toEqual(new data.BooleanData(false));
|
||||||
|
// job-index and job-total are runtime values, not specified in YAML
|
||||||
|
expect(context.get("job-index")).toEqual(new data.NumberData(0));
|
||||||
|
expect(context.get("job-total")).toEqual(new data.NumberData(1));
|
||||||
|
expect(context.get("max-parallel")).toEqual(new data.NumberData(3));
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -3,15 +3,24 @@ import {isMapping, isScalar, isString} from "@actions/workflow-parser";
|
|||||||
import {WorkflowContext} from "../context/workflow-context";
|
import {WorkflowContext} from "../context/workflow-context";
|
||||||
import {scalarToData} from "../utils/scalar-to-data";
|
import {scalarToData} from "../utils/scalar-to-data";
|
||||||
|
|
||||||
|
// Default strategy values when no strategy block is defined
|
||||||
|
const DEFAULT_STRATEGY = {
|
||||||
|
"fail-fast": new data.BooleanData(true),
|
||||||
|
"job-index": new data.NumberData(0),
|
||||||
|
"job-total": new data.NumberData(1),
|
||||||
|
"max-parallel": new data.NumberData(1)
|
||||||
|
};
|
||||||
|
|
||||||
export function getStrategyContext(workflowContext: WorkflowContext): DescriptionDictionary {
|
export function getStrategyContext(workflowContext: WorkflowContext): DescriptionDictionary {
|
||||||
// https://docs.github.com/en/actions/learn-github-actions/contexts#strategy-context
|
// https://docs.github.com/en/actions/learn-github-actions/contexts#strategy-context
|
||||||
const keys = ["fail-fast", "job-index", "job-total", "max-parallel"];
|
const keys = ["fail-fast", "job-index", "job-total", "max-parallel"];
|
||||||
|
|
||||||
const strategy = workflowContext.job?.strategy ?? workflowContext.reusableWorkflowJob?.strategy;
|
const strategy = workflowContext.job?.strategy ?? workflowContext.reusableWorkflowJob?.strategy;
|
||||||
if (!strategy || !isMapping(strategy)) {
|
if (!strategy || !isMapping(strategy)) {
|
||||||
|
// No strategy defined - return defaults that match runtime behavior
|
||||||
return new DescriptionDictionary(
|
return new DescriptionDictionary(
|
||||||
...keys.map(key => {
|
...keys.map(key => {
|
||||||
return {key, value: new data.Null()};
|
return {key, value: DEFAULT_STRATEGY[key as keyof typeof DEFAULT_STRATEGY]};
|
||||||
})
|
})
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
@@ -31,7 +40,8 @@ export function getStrategyContext(workflowContext: WorkflowContext): Descriptio
|
|||||||
|
|
||||||
for (const key of keys) {
|
for (const key of keys) {
|
||||||
if (!strategyContext.get(key)) {
|
if (!strategyContext.get(key)) {
|
||||||
strategyContext.add(key, new data.Null());
|
// Use default value for missing properties
|
||||||
|
strategyContext.add(key, DEFAULT_STRATEGY[key as keyof typeof DEFAULT_STRATEGY]);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -681,7 +681,8 @@ jobs:
|
|||||||
|
|
||||||
const result = await validate(createDocument("wf.yaml", input));
|
const result = await validate(createDocument("wf.yaml", input));
|
||||||
|
|
||||||
expect(result).not.toEqual([]);
|
// Strategy context is always available with default values
|
||||||
|
expect(result).toEqual([]);
|
||||||
});
|
});
|
||||||
|
|
||||||
it("invalid strategy property", async () => {
|
it("invalid strategy property", async () => {
|
||||||
@@ -996,22 +997,8 @@ jobs:
|
|||||||
|
|
||||||
const result = await validate(createDocument("wf.yaml", input));
|
const result = await validate(createDocument("wf.yaml", input));
|
||||||
|
|
||||||
expect(result).toEqual([
|
// Matrix is null when no strategy is defined, accessing properties on null is valid
|
||||||
{
|
expect(result).toEqual([]);
|
||||||
message: "Context access might be invalid: matrix",
|
|
||||||
range: {
|
|
||||||
end: {
|
|
||||||
character: 36,
|
|
||||||
line: 8
|
|
||||||
},
|
|
||||||
start: {
|
|
||||||
character: 18,
|
|
||||||
line: 8
|
|
||||||
}
|
|
||||||
},
|
|
||||||
severity: DiagnosticSeverity.Warning
|
|
||||||
}
|
|
||||||
]);
|
|
||||||
});
|
});
|
||||||
|
|
||||||
it("basic matrix", async () => {
|
it("basic matrix", async () => {
|
||||||
|
|||||||
Reference in New Issue
Block a user