Skip to content

Commit

Permalink
fix: relax PR description validation (#6964)
Browse files Browse the repository at this point in the history
  • Loading branch information
vicb authored Oct 15, 2024
1 parent f996409 commit 9b5910f
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 14 deletions.
40 changes: 37 additions & 3 deletions tools/deployments/__tests__/validate-pr-description.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ describe("validateDescription()", () => {
it("should skip validation with the `skip-pr-description-validation` label", () => {
expect(
validateDescription("", "", '["skip-pr-description-validation"]')
).toMatchInlineSnapshot(`[]`);
).toHaveLength(0);
});

it("should show errors with default template + TODOs checked", () => {
expect(
validateDescription(
Expand Down Expand Up @@ -76,7 +77,7 @@ Fixes [AA-000](https://jira.cfdata.org/browse/AA-000).
`,
"[]"
)
).toMatchInlineSnapshot(`[]`);
).toHaveLength(0);
});

it("should not accept e2e unknown", () => {
Expand Down Expand Up @@ -151,6 +152,7 @@ Fixes [AA-000](https://jira.cfdata.org/browse/AA-000).
]
`);
});

it("should accept e2e with e2e label", () => {
expect(
validateDescription(
Expand Down Expand Up @@ -180,6 +182,38 @@ Fixes [AA-000](https://jira.cfdata.org/browse/AA-000).
`,
'["e2e"]'
)
).toMatchInlineSnapshot(`[]`);
).toHaveLength(0);
});

it("should accept e2e with e2e label - uppercase X", () => {
expect(
validateDescription(
"",
`## What this PR solves / how to test
Fixes [AA-000](https://jira.cfdata.org/browse/AA-000).
## Author has addressed the following
- Tests
- [ ] TODO (before merge)
- [X] Tests included
- [ ] Tests not necessary because:
- E2E Tests CI Job required? (Use "e2e" label or ask maintainer to run separately)
- [ ] I don't know
- [X] Required
- [ ] Not required because: test
- Changeset ([Changeset guidelines](https://github.com/cloudflare/workers-sdk/blob/main/CONTRIBUTING.md#changesets))
- [ ] TODO (before merge)
- [X] Changeset included
- [ ] Changeset not necessary because:
- Public documentation
- [ ] TODO (before merge)
- [X] Cloudflare docs PR(s): https://github.com/cloudflare/cloudflare-docs/pull/123
- [ ] Documentation not necessary because:
`,
'["e2e"]'
)
).toHaveLength(0);
});
});
22 changes: 11 additions & 11 deletions tools/deployments/validate-pr-description.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,50 +31,50 @@ export function validateDescription(
return [];
}

if (/- \[x\] TODO \(before merge\)/.test(body)) {
if (/- \[x\] TODO \(before merge\)/i.test(body)) {
errors.push(
"All TODO checkboxes in your PR description must be unchecked before merging"
);
}

if (
!(
/- \[x\] Tests included/.test(body) ||
/- \[x\] Tests not necessary because: .+/.test(body)
/- \[x\] Tests included/i.test(body) ||
/- \[x\] Tests not necessary because: .+/i.test(body)
)
) {
errors.push(
"Your PR must include tests, or provide justification for why no tests are required"
);
}

if (/- \[x\] I don't know/.test(body)) {
if (/- \[x\] I don't know/i.test(body)) {
errors.push(
"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"
);
}

if (
!(
/- \[x\] Required/.test(body) ||
/- \[x\] Not required because: .+/.test(body)
/- \[x\] Required/i.test(body) ||
/- \[x\] Not required because: .+/i.test(body)
)
) {
errors.push(
"Your PR must run E2E tests, or provide justification for why running them is not required"
);
}

if (/- \[x\] Required/.test(body) && !parsedLabels.includes("e2e")) {
if (/- \[x\] Required/i.test(body) && !parsedLabels.includes("e2e")) {
errors.push(
"Since your PR requires E2E tests to be run, it needs to have the `e2e` label applied on GitHub"
);
}

if (
!(
/- \[x\] Changeset included/.test(body) ||
/- \[x\] Changeset not necessary because: .+/.test(body)
/- \[x\] Changeset included/i.test(body) ||
/- \[x\] Changeset not necessary because: .+/i.test(body)
)
) {
errors.push(
Expand All @@ -84,9 +84,9 @@ export function validateDescription(

if (
!(
/- \[x\] Cloudflare docs PR\(s\): https:\/\/github\.com\/cloudflare\/cloudflare-docs\/(pull|issues)\/\d+/.test(
/- \[x\] Cloudflare docs PR\(s\): https:\/\/github\.com\/cloudflare\/cloudflare-docs\/(pull|issues)\/\d+/i.test(
body
) || /- \[x\] Documentation not necessary because: .+/.test(body)
) || /- \[x\] Documentation not necessary because: .+/i.test(body)
)
) {
errors.push(
Expand Down

0 comments on commit 9b5910f

Please sign in to comment.