Skip to content

Commit 9b6a74a

Browse files
penalosapenalosa
authored
Validate changeset type & PR description (#6683)
1 parent 66f5b25 commit 9b6a74a

File tree

6 files changed

+449
-12
lines changed

6 files changed

+449
-12
lines changed

‎.github/pull_request_template.md‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,20 +6,20 @@ Fixes #[insert GH or internal issue number(s)].
66

77
- Tests
88
- [ ] TODO (before merge)
9-
- [ ] Included
10-
- [ ] Not necessary because:
9+
- [ ] Tests included
10+
- [ ] Tests not necessary because:
1111
- E2E Tests CI Job required? (Use "e2e" label or ask maintainer to run separately)
1212
- [ ] I don't know
13-
- [ ] Required / Maybe required
14-
- [ ] Not required because:
13+
- [ ] Required
14+
- [ ] Not required because:
1515
- Changeset ([Changeset guidelines](https://github.com/cloudflare/workers-sdk/blob/main/CONTRIBUTING.md#changesets))
1616
- [ ] TODO (before merge)
17-
- [ ] Included
18-
- [ ] Not necessary because:
17+
- [ ] Changeset included
18+
- [ ] Changeset not necessary because:
1919
- Public documentation
2020
- [ ] TODO (before merge)
2121
- [ ] Cloudflare docs PR(s): <!--e.g. <https://github.com/cloudflare/cloudflare-docs/pull/>...-->
22-
- [ ] Not necessary because:
22+
- [ ] Documentation not necessary because:
2323

2424
<!--
2525
Have you read our [Contributing guide](https://github.com/cloudflare/workers-sdk/blob/main/CONTRIBUTING.md)?

‎.github/workflows/validate-pr-description.yml‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
name: Validate PR Description
2+
3+
on:
4+
pull_request:
5+
types: [opened, synchronize, reopened, labeled, unlabeled, edited]
6+
7+
jobs:
8+
check:
9+
if: github.ref != 'refs/heads/changeset-release/main'
10+
concurrency:
11+
group: ${{ github.workflow }}-${{ github.ref }}-add-pr
12+
cancel-in-progress: true
13+
timeout-minutes: 30
14+
name: Check
15+
runs-on: ubuntu-latest
16+
steps:
17+
- name: Checkout Repo
18+
uses: actions/checkout@v4
19+
with:
20+
fetch-depth: 0
21+
22+
- name: Install Dependencies
23+
uses: ./.github/actions/install-dependencies
24+
with:
25+
turbo-api: ${{ secrets.TURBO_API }}
26+
turbo-team: ${{ secrets.TURBO_TEAM }}
27+
turbo-token: ${{ secrets.TURBO_TOKEN }}
28+
turbo-signature: ${{ secrets.TURBO_REMOTE_CACHE_SIGNATURE_KEY }}
29+
30+
- run: node -r esbuild-register tools/deployments/validate-pr-description.ts
31+
env:
32+
TITLE: ${{ github.event.pull_request.title }}
33+
BODY: ${{ github.event.pull_request.body }}
34+
LABELS: ${{ toJson(github.event.pull_request.labels.*.name) }}

‎tools/deployments/__tests__/validate-changesets.test.ts‎

Lines changed: 44 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -95,41 +95,80 @@ describe("validateChangesets()", () => {
9595
contents: dedent`
9696
---
9797
"package-a": patch
98-
---`,
98+
---
99+
100+
refactor: test`,
99101
},
100102
{
101103
file: "valid-two.md",
102104
contents: dedent`
103105
---
104106
"package-b": minor
105-
---`,
107+
---
108+
109+
feature: test`,
110+
},
111+
{
112+
file: "valid-three.md",
113+
contents: dedent`
114+
---
115+
"package-c": major
116+
---
117+
118+
chore: test`,
119+
},
120+
{
121+
file: "invalid-changetype-one.md",
122+
contents: dedent`
123+
---
124+
"package-a": patch
125+
---
126+
127+
random: test`,
128+
},
129+
{
130+
file: "invalid-changetype-two.md",
131+
contents: dedent`
132+
---
133+
"package-b": minor
134+
---
135+
136+
change: test`,
106137
},
107138
{
108139
file: "valid-three.md",
109140
contents: dedent`
110141
---
111142
"package-c": major
112-
---`,
143+
---
144+
145+
fix: test`,
113146
},
114147
{ file: "invalid-frontmatter.md", contents: "" },
115148
{
116149
file: "invalid-package.md",
117150
contents: dedent`
118151
---
119152
"package-invalid": major
120-
---`,
153+
---
154+
155+
feat: test`,
121156
},
122157
{
123158
file: "invalid-type.md",
124159
contents: dedent`
125160
---
126161
"package-a": foo
127-
---`,
162+
---
163+
164+
docs: test`,
128165
},
129166
]
130167
);
131168
expect(errors).toMatchInlineSnapshot(`
132169
[
170+
"Invalid summary in changeset "invalid-changetype-one.md". It must start with one of "feat:", "fix:", "refactor:", "docs:", or "chore:"",
171+
"Invalid summary in changeset "invalid-changetype-two.md". It must start with one of "feat:", "fix:", "refactor:", "docs:", or "chore:"",
133172
"Error: could not parse changeset - invalid frontmatter: at file "invalid-frontmatter.md"",
134173
"Invalid package name "package-invalid" in changeset at "invalid-package.md".",
135174
"Invalid type "foo" for package "package-a" in changeset at "invalid-type.md".",

‎tools/deployments/__tests__/validate-pr-description.test.ts‎

Lines changed: 249 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,249 @@
1+
import { describe, expect, it } from "vitest";
2+
import { validateDescription } from "../validate-pr-description";
3+
4+
describe("validateDescription()", () => {
5+
it("should skip validation with the `skip-pr-validation` label", () => {
6+
expect(
7+
validateDescription("", "", '["skip-pr-validation"]')
8+
).toMatchInlineSnapshot(`[]`);
9+
});
10+
it("should show errors with default template + TODOs checked", () => {
11+
expect(
12+
validateDescription(
13+
"",
14+
`## What this PR solves / how to test
15+
16+
Fixes #[insert GH or internal issue number(s)].
17+
18+
## Author has addressed the following
19+
20+
- Tests
21+
- [ ] TODO (before merge)
22+
- [ ] Tests included
23+
- [ ] Tests not necessary because:
24+
- E2E Tests CI Job required? (Use "e2e" label or ask maintainer to run separately)
25+
- [ ] I don't know
26+
- [ ] Required
27+
- [ ] Not required because:
28+
- Changeset ([Changeset guidelines](https://github.com/cloudflare/workers-sdk/blob/main/CONTRIBUTING.md#changesets))
29+
- [ ] TODO (before merge)
30+
- [ ] Changeset included
31+
- [ ] Changeset not necessary because:
32+
- Public documentation
33+
- [x] TODO (before merge)
34+
- [ ] Cloudflare docs PR(s): <!--e.g. <https://github.com/cloudflare/cloudflare-docs/pull/>...-->
35+
- [ ] Documentation not necessary because:
36+
`,
37+
"[]"
38+
)
39+
).toMatchInlineSnapshot(`
40+
[
41+
"Your PR description must include an issue reference in the format \`Fixes #000\` (for GitHub issues) or \`Fixes [AA-000](https://jira.cfdata.org/browse/AA-000)\` (for internal Jira ticket references)",
42+
"All TODO checkboxes in your PR description must be unchecked before merging",
43+
"Your PR must include tests, or provide justification for why no tests are required",
44+
"Your PR must run E2E tests, or provide justification for why running them is not required",
45+
"Your PR must include a changeset, or provide justification for why no changesets are required",
46+
"Your PR must include documentation (in the form of a link to a Cloudflare Docs issue or PR), or provide justification for why no documentation is required",
47+
]
48+
`);
49+
});
50+
it("should accept GitHub issue reference", () => {
51+
expect(
52+
validateDescription(
53+
"",
54+
`## What this PR solves / how to test
55+
56+
Fixes #1234.
57+
58+
## Author has addressed the following
59+
60+
- Tests
61+
- [ ] TODO (before merge)
62+
- [ ] Tests included
63+
- [x] Tests not necessary because: test
64+
- E2E Tests CI Job required? (Use "e2e" label or ask maintainer to run separately)
65+
- [ ] I don't know
66+
- [ ] Required
67+
- [x] Not required because: test
68+
- Changeset ([Changeset guidelines](https://github.com/cloudflare/workers-sdk/blob/main/CONTRIBUTING.md#changesets))
69+
- [ ] TODO (before merge)
70+
- [ ] Changeset included
71+
- [x] Changeset not necessary because: test
72+
- Public documentation
73+
- [ ] TODO (before merge)
74+
- [ ] Cloudflare docs PR(s): <!--e.g. <https://github.com/cloudflare/cloudflare-docs/pull/>...-->
75+
- [x] Documentation not necessary because: test
76+
`,
77+
"[]"
78+
)
79+
).toMatchInlineSnapshot(`[]`);
80+
});
81+
82+
it("should accept JIRA issue reference", () => {
83+
expect(
84+
validateDescription(
85+
"",
86+
`## What this PR solves / how to test
87+
88+
Fixes [AA-000](https://jira.cfdata.org/browse/AA-000).
89+
90+
## Author has addressed the following
91+
92+
- Tests
93+
- [ ] TODO (before merge)
94+
- [ ] Tests included
95+
- [x] Tests not necessary because: test
96+
- E2E Tests CI Job required? (Use "e2e" label or ask maintainer to run separately)
97+
- [ ] I don't know
98+
- [ ] Required
99+
- [x] Not required because: test
100+
- Changeset ([Changeset guidelines](https://github.com/cloudflare/workers-sdk/blob/main/CONTRIBUTING.md#changesets))
101+
- [ ] TODO (before merge)
102+
- [ ] Changeset included
103+
- [x] Changeset not necessary because: test
104+
- Public documentation
105+
- [ ] TODO (before merge)
106+
- [ ] Cloudflare docs PR(s): <!--e.g. <https://github.com/cloudflare/cloudflare-docs/pull/>...-->
107+
- [x] Documentation not necessary because: test
108+
`,
109+
"[]"
110+
)
111+
).toMatchInlineSnapshot(`[]`);
112+
});
113+
114+
it("should accept everything included", () => {
115+
expect(
116+
validateDescription(
117+
"",
118+
`## What this PR solves / how to test
119+
120+
Fixes [AA-000](https://jira.cfdata.org/browse/AA-000).
121+
122+
## Author has addressed the following
123+
124+
- Tests
125+
- [ ] TODO (before merge)
126+
- [x] Tests included
127+
- [ ] Tests not necessary because:
128+
- E2E Tests CI Job required? (Use "e2e" label or ask maintainer to run separately)
129+
- [ ] I don't know
130+
- [ ] Required
131+
- [x] Not required because: test
132+
- Changeset ([Changeset guidelines](https://github.com/cloudflare/workers-sdk/blob/main/CONTRIBUTING.md#changesets))
133+
- [ ] TODO (before merge)
134+
- [x] Changeset included
135+
- [ ] Changeset not necessary because:
136+
- Public documentation
137+
- [ ] TODO (before merge)
138+
- [x] Cloudflare docs PR(s): https://github.com/cloudflare/cloudflare-docs/pull/123
139+
- [ ] Documentation not necessary because:
140+
`,
141+
"[]"
142+
)
143+
).toMatchInlineSnapshot(`[]`);
144+
});
145+
146+
it("should not accept e2e unknown", () => {
147+
expect(
148+
validateDescription(
149+
"",
150+
`## What this PR solves / how to test
151+
152+
Fixes [AA-000](https://jira.cfdata.org/browse/AA-000).
153+
154+
## Author has addressed the following
155+
156+
- Tests
157+
- [ ] TODO (before merge)
158+
- [x] Tests included
159+
- [ ] Tests not necessary because:
160+
- E2E Tests CI Job required? (Use "e2e" label or ask maintainer to run separately)
161+
- [x] I don't know
162+
- [ ] Required
163+
- [ ] Not required because: test
164+
- Changeset ([Changeset guidelines](https://github.com/cloudflare/workers-sdk/blob/main/CONTRIBUTING.md#changesets))
165+
- [ ] TODO (before merge)
166+
- [x] Changeset included
167+
- [ ] Changeset not necessary because:
168+
- Public documentation
169+
- [ ] TODO (before merge)
170+
- [x] Cloudflare docs PR(s): https://github.com/cloudflare/cloudflare-docs/pull/123
171+
- [ ] Documentation not necessary because:
172+
`,
173+
"[]"
174+
)
175+
).toMatchInlineSnapshot(`
176+
[
177+
"Your PR cannot be merged with a status of \`I don't know\` for e2e tests. When your PR is reviewed by the Wrangler team they'll decide whether e2e tests need to be run",
178+
"Your PR must run E2E tests, or provide justification for why running them is not required",
179+
]
180+
`);
181+
});
182+
183+
it("should not accept e2e without e2e label", () => {
184+
expect(
185+
validateDescription(
186+
"",
187+
`## What this PR solves / how to test
188+
189+
Fixes [AA-000](https://jira.cfdata.org/browse/AA-000).
190+
191+
## Author has addressed the following
192+
193+
- Tests
194+
- [ ] TODO (before merge)
195+
- [x] Tests included
196+
- [ ] Tests not necessary because:
197+
- E2E Tests CI Job required? (Use "e2e" label or ask maintainer to run separately)
198+
- [ ] I don't know
199+
- [x] Required
200+
- [ ] Not required because: test
201+
- Changeset ([Changeset guidelines](https://github.com/cloudflare/workers-sdk/blob/main/CONTRIBUTING.md#changesets))
202+
- [ ] TODO (before merge)
203+
- [x] Changeset included
204+
- [ ] Changeset not necessary because:
205+
- Public documentation
206+
- [ ] TODO (before merge)
207+
- [x] Cloudflare docs PR(s): https://github.com/cloudflare/cloudflare-docs/pull/123
208+
- [ ] Documentation not necessary because:
209+
`,
210+
"[]"
211+
)
212+
).toMatchInlineSnapshot(`
213+
[
214+
"Since your PR requires E2E tests to be run, it needs to have the \`e2e\` label applied on GitHub",
215+
]
216+
`);
217+
});
218+
it("should accept e2e with e2e label", () => {
219+
expect(
220+
validateDescription(
221+
"",
222+
`## What this PR solves / how to test
223+
224+
Fixes [AA-000](https://jira.cfdata.org/browse/AA-000).
225+
226+
## Author has addressed the following
227+
228+
- Tests
229+
- [ ] TODO (before merge)
230+
- [x] Tests included
231+
- [ ] Tests not necessary because:
232+
- E2E Tests CI Job required? (Use "e2e" label or ask maintainer to run separately)
233+
- [ ] I don't know
234+
- [x] Required
235+
- [ ] Not required because: test
236+
- Changeset ([Changeset guidelines](https://github.com/cloudflare/workers-sdk/blob/main/CONTRIBUTING.md#changesets))
237+
- [ ] TODO (before merge)
238+
- [x] Changeset included
239+
- [ ] Changeset not necessary because:
240+
- Public documentation
241+
- [ ] TODO (before merge)
242+
- [x] Cloudflare docs PR(s): https://github.com/cloudflare/cloudflare-docs/pull/123
243+
- [ ] Documentation not necessary because:
244+
`,
245+
'["e2e"]'
246+
)
247+
).toMatchInlineSnapshot(`[]`);
248+
});
249+
});

0 commit comments

Comments
 (0)