Skip to content

[jsweep] Clean add_labels.cjs #35134

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 3 commits into from
May 27, 2026
+111 −0
Merged

[jsweep] Clean add_labels.cjs #35134

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f32e838
test(jsweep): improve add_labels.cjs test coverage
github-actions[bot] May 27, 2026
9eb51a2
chore: outline plan for review feedback
Copilot May 27, 2026
33c9269
test: add missing combined filter coverage for add_labels
Copilot May 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 111 additions & 0 deletions actions/setup/js/add_labels.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ describe("add_labels", () => {
rest: {
issues: {
addLabels: async () => ({}),
get: async () => ({
data: {
title: "Test issue title",
labels: [],
},
}),
},
},
};
Expand Down Expand Up @@ -858,5 +864,110 @@ describe("add_labels", () => {
expect(result.success).toBe(false);
expect(result.error).toContain("No issue/PR number available");
});

it("should skip when item does not have all required_labels", async () => {
const handler = await main({ max: 10, required_labels: ["needs-triage"] });

mockGithub.rest.issues.get = async () => ({
data: { title: "Some issue", labels: [{ name: "bug" }] },
Comment on lines +868 to +872
});

const result = await handler({ item_number: 100, labels: ["enhancement"] }, {});

expect(result.success).toBe(false);
expect(result.skipped).toBe(true);
expect(result.error).toContain("required-labels");
});

it("should add labels when item has all required_labels", async () => {
const handler = await main({ max: 10, required_labels: ["needs-triage"] });
const addLabelsCalls = [];

mockGithub.rest.issues.get = async () => ({
data: { title: "Some issue", labels: [{ name: "needs-triage" }, { name: "bug" }] },
});
mockGithub.rest.issues.addLabels = async params => {
addLabelsCalls.push(params);
return {};
};

Copy link
Copy Markdown
Contributor Author

@github-actions github-actions Bot May 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/tdd] The assertion only checks that addLabels was called once, not which labels were sent — a bug passing the wrong labels would still pass this test.

💡 Suggested addition 
expect(addLabelsCalls[0].labels).toEqual(["enhancement"]);

This makes the test a true specification: it verifies both that the action was taken and that the correct payload was used.

const result = await handler({ item_number: 100, labels: ["enhancement"] }, {});

expect(result.success).toBe(true);
expect(addLabelsCalls.length).toBe(1);
});

it("should skip when item title does not start with required_title_prefix", async () => {
const handler = await main({ max: 10, required_title_prefix: "[Bot]" });

mockGithub.rest.issues.get = async () => ({
data: { title: "Regular issue title", labels: [] },
});

const result = await handler({ item_number: 100, labels: ["bug"] }, {});

expect(result.success).toBe(false);
expect(result.skipped).toBe(true);
expect(result.error).toContain("required prefix");
});

it("should add labels when item title starts with required_title_prefix", async () => {
const handler = await main({ max: 10, required_title_prefix: "[Bot]" });
const addLabelsCalls = [];

mockGithub.rest.issues.get = async () => ({
data: { title: "[Bot] Automated issue", labels: [] },
});
mockGithub.rest.issues.addLabels = async params => {
addLabelsCalls.push(params);
return {};
};
Copy link
Copy Markdown
Contributor Author

@github-actions github-actions Bot May 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/tdd] Same as above — addLabelsCalls.length === 1 confirms invocation but not the payload.

💡 Suggested addition 
expect(addLabelsCalls[0].labels).toEqual(["automation"]);


const result = await handler({ item_number: 100, labels: ["automation"] }, {});

expect(result.success).toBe(true);
expect(addLabelsCalls.length).toBe(1);
});

it("should check both required_labels and required_title_prefix together", async () => {
const handler = await main({
max: 10,
required_labels: ["approved"],
required_title_prefix: "[Ready]",
});

// Passes required_labels but fails required_title_prefix
mockGithub.rest.issues.get = async () => ({
data: { title: "Not ready issue", labels: [{ name: "approved" }] },
});

const result = await handler({ item_number: 100, labels: ["bug"] }, {});
Copy link
Copy Markdown
Contributor Author

@github-actions github-actions Bot May 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/tdd] The combined-filter test only covers the "passes labels, fails prefix" path. Consider adding a case where both conditions are satisfied to confirm the happy path works end-to-end with both filters active.

💡 Sketch 
it("should add labels when both required_labels and required_title_prefix match", async () => {
  const handler = await main({
    max: 10,
    required_labels: ["approved"],
    required_title_prefix: "[Ready]",
  });
  const addLabelsCalls = [];

  mockGithub.rest.issues.get = async () => ({
    data: { title: "[Ready] Deploy v2", labels: [{ name: "approved" }] },
  });
  mockGithub.rest.issues.addLabels = async params => {
    addLabelsCalls.push(params);
    return {};
  };

  const result = await handler({ item_number: 100, labels: ["deploy"] }, {});

  expect(result.success).toBe(true);
  expect(addLabelsCalls.length).toBe(1);
});


expect(result.success).toBe(false);
expect(result.skipped).toBe(true);
expect(result.error).toContain("required prefix");
});
Copy link
Copy Markdown
Contributor Author

@github-actions github-actions Bot May 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Combined-filter test is missing the "both conditions pass" happy path, leaving a gap in AND-semantics coverage.

💡 Details

The only combined-config test covers: required_labels ✅ + required_title_prefix ❌ → skipped.

Two paths are untested under the combined config:

  1. labels-fail, prefix-pass — a production bug that skips the labels check when a prefix is also configured would go undetected (the standalone required_labels test uses a config without a prefix, so it can't catch combined-mode regressions).
  2. both pass — no combined-config test verifies that labels are actually added when both filters are satisfied. A bug that erroneously returns skipped: true in the combined path would pass all current tests.

Suggested addition:

it("should add labels when both required_labels and required_title_prefix are satisfied", async () => {
  const handler = await main({
    max: 10,
    required_labels: ["approved"],
    required_title_prefix: "[Ready]",
  });
  mockGithub.rest.issues.get = async () => ({
    data: { title: "[Ready] Approved issue", labels: [{ name: "approved" }] },
  });
  const result = await handler({ item_number: 100, labels: ["bug"] }, {});
  expect(result.success).toBe(true);
  expect(result.skipped).toBeUndefined();
});


it("should add labels when both required_labels and required_title_prefix match", async () => {
const handler = await main({
max: 10,
required_labels: ["approved"],
required_title_prefix: "[Ready]",
});
const addLabelsCalls = [];

mockGithub.rest.issues.get = async () => ({
data: { title: "[Ready] Ship it", labels: [{ name: "approved" }, { name: "bug" }] },
});
mockGithub.rest.issues.addLabels = async params => {
addLabelsCalls.push(params);
return {};
};

const result = await handler({ item_number: 100, labels: ["enhancement"] }, {});

expect(result.success).toBe(true);
expect(addLabelsCalls.length).toBe(1);
});
});
});