Skip to content

Commit 4c0615c

Browse files
koki-developkoki-develop
authored
fix: Prevent adding duplicate job IDs (#81)
1 parent 6ffd277 commit 4c0615c

File tree

2 files changed

+131
-93
lines changed

2 files changed

+131
-93
lines changed

‎lib/package/workflow.spec.ts‎

Lines changed: 121 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -3,96 +3,130 @@ import { Job } from "./job";
33
import { Workflow } from "./workflow";
44

55
describe("Workflow", () => {
6-
test.each<[Workflow, Record<string, unknown>]>([
7-
[
8-
new Workflow("simple", { on: "push" }),
9-
{ name: "simple", on: "push", jobs: {} },
10-
],
11-
[
12-
new Workflow("with run-name", { runName: "Simple Workflow", on: "push" }),
13-
{
14-
name: "with run-name",
15-
"run-name": "Simple Workflow",
16-
on: "push",
17-
jobs: {},
18-
},
19-
],
20-
[
21-
new Workflow("with permissions", {
22-
on: "push",
23-
permissions: { contents: "read" },
24-
}),
25-
{
26-
name: "with permissions",
27-
on: "push",
28-
permissions: { contents: "read" },
29-
jobs: {},
30-
},
31-
],
32-
[
33-
new Workflow("with env", {
34-
on: "push",
35-
env: { FOO: "foo", BAR: "bar" },
36-
}),
37-
{
38-
name: "with env",
39-
on: "push",
40-
env: {
41-
FOO: "foo",
42-
BAR: "bar",
6+
describe("toJSON", () => {
7+
test.each<[Workflow, Record<string, unknown>]>([
8+
[
9+
new Workflow("simple", { on: "push" }),
10+
{ name: "simple", on: "push", jobs: {} },
11+
],
12+
[
13+
new Workflow("with run-name", {
14+
runName: "Simple Workflow",
15+
on: "push",
16+
}),
17+
{
18+
name: "with run-name",
19+
"run-name": "Simple Workflow",
20+
on: "push",
21+
jobs: {},
4322
},
44-
jobs: {},
45-
},
46-
],
47-
[
48-
new Workflow("with concurrency", {
49-
on: "push",
50-
concurrency: { group: "group", cancelInProgress: true },
51-
}),
52-
{
53-
name: "with concurrency",
54-
on: "push",
55-
concurrency: {
56-
group: "group",
57-
"cancel-in-progress": true,
23+
],
24+
[
25+
new Workflow("with permissions", {
26+
on: "push",
27+
permissions: { contents: "read" },
28+
}),
29+
{
30+
name: "with permissions",
31+
on: "push",
32+
permissions: { contents: "read" },
33+
jobs: {},
5834
},
59-
jobs: {},
60-
},
61-
],
62-
[
63-
new Workflow("with defaults", {
64-
on: "push",
65-
defaults: { run: { shell: "bash" } },
66-
}),
67-
{
68-
name: "with defaults",
69-
on: "push",
70-
defaults: { run: { shell: "bash" } },
71-
jobs: {},
72-
},
73-
],
74-
[
75-
(() => {
76-
const workflow = new Workflow("with job", { on: "push" });
77-
workflow.addJob(
78-
new Job("simple-job", {
79-
runsOn: "ubuntu-latest",
80-
}).run("echo 'Hello, world!'"),
81-
);
82-
return workflow;
83-
})(),
84-
{
85-
name: "with job",
86-
on: "push",
87-
jobs: {
88-
"simple-job": {
89-
"runs-on": "ubuntu-latest",
90-
steps: [{ run: "echo 'Hello, world!'" }],
35+
],
36+
[
37+
new Workflow("with env", {
38+
on: "push",
39+
env: { FOO: "foo", BAR: "bar" },
40+
}),
41+
{
42+
name: "with env",
43+
on: "push",
44+
env: {
45+
FOO: "foo",
46+
BAR: "bar",
47+
},
48+
jobs: {},
49+
},
50+
],
51+
[
52+
new Workflow("with concurrency", {
53+
on: "push",
54+
concurrency: { group: "group", cancelInProgress: true },
55+
}),
56+
{
57+
name: "with concurrency",
58+
on: "push",
59+
concurrency: {
60+
group: "group",
61+
"cancel-in-progress": true,
62+
},
63+
jobs: {},
64+
},
65+
],
66+
[
67+
new Workflow("with defaults", {
68+
on: "push",
69+
defaults: { run: { shell: "bash" } },
70+
}),
71+
{
72+
name: "with defaults",
73+
on: "push",
74+
defaults: { run: { shell: "bash" } },
75+
jobs: {},
76+
},
77+
],
78+
[
79+
(() => {
80+
const workflow = new Workflow("with job", { on: "push" });
81+
workflow.addJob(
82+
new Job("simple-job-1", {
83+
runsOn: "ubuntu-latest",
84+
}).run("echo 'Hello, 1!'"),
85+
);
86+
workflow.addJob(
87+
new Job("simple-job-2", {
88+
runsOn: "ubuntu-latest",
89+
}).run("echo 'Hello, 2!'"),
90+
);
91+
return workflow;
92+
})(),
93+
{
94+
name: "with job",
95+
on: "push",
96+
jobs: {
97+
"simple-job-1": {
98+
"runs-on": "ubuntu-latest",
99+
steps: [{ run: "echo 'Hello, 1!'" }],
100+
},
101+
"simple-job-2": {
102+
"runs-on": "ubuntu-latest",
103+
steps: [{ run: "echo 'Hello, 2!'" }],
104+
},
91105
},
92106
},
93-
},
94-
],
95-
])("workflow.toJSON() -> %j", (workflow, expected) => {
96-
expect(workflow.toJSON()).toEqual(expected);
107+
],
108+
])("workflow.toJSON() -> %j", (workflow, expected) => {
109+
expect(workflow.toJSON()).toEqual(expected);
110+
});
111+
});
112+
113+
describe("addJob", () => {
114+
test("should throw an error if a job with the same id already exists (1)", () => {
115+
const workflow = new Workflow("simple", { on: "push" });
116+
workflow.addJob(new Job("simple-job", { runsOn: "ubuntu-latest" }));
117+
expect(() =>
118+
workflow.addJob(new Job("simple-job", { runsOn: "ubuntu-latest" })),
119+
).toThrow();
120+
});
121+
122+
test("should throw an error if a job with the same id already exists (2)", () => {
123+
const workflow = new Workflow("simple", { on: "push" });
124+
expect(() =>
125+
workflow.addJob(
126+
new Job("simple-job", { runsOn: "ubuntu-latest" }),
127+
new Job("simple-job", { runsOn: "ubuntu-latest" }),
128+
),
129+
).toThrow();
130+
});
97131
});
98132
});

‎lib/package/workflow.ts‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,15 +46,20 @@ export type WorkflowConfig = {
4646
export class Workflow {
4747
private readonly _name: string;
4848
private readonly _config: WorkflowConfig;
49-
private readonly _jobs: Job[] = [];
49+
private readonly _jobs: Map<string, Job> = new Map();
5050

5151
public constructor(name: string, config: WorkflowConfig) {
5252
this._name = name;
5353
this._config = config;
5454
}
5555

5656
public addJob(...jobs: Job[]): Workflow {
57-
this._jobs.push(...jobs);
57+
for (const job of jobs) {
58+
if (this._jobs.has(job.id)) {
59+
throw new Error(`Job ${job.id} already exists`);
60+
}
61+
this._jobs.set(job.id, job);
62+
}
5863
return this;
5964
}
6065

@@ -80,10 +85,9 @@ export class Workflow {
8085
defaults: defaultsJSON(this._config.defaults),
8186
}),
8287

83-
jobs: this._jobs.reduce<Record<string, unknown>>((acc, job) => {
84-
acc[job.id] = job.toJSON();
85-
return acc;
86-
}, {}),
88+
jobs: Object.fromEntries(
89+
Array.from(this._jobs.entries()).map(([id, job]) => [id, job.toJSON()]),
90+
),
8791
};
8892
}
8993
}

0 commit comments

Comments
 (0)