feat: add unit-testing skill (Osherove 3rd Ed)

[royosherove]

Entry/exit points, trust boundaries, AAA, stubs/mocks/fakes, anti-patterns.

[github-actions]

Claude finished @royosherove's task in 1m 2s —— View job

---

PR Review: unit-testing/SKILL.md

• Gather context (read CLAUDE.md, changed files)
• Review unit-testing/SKILL.md
• Post review feedback

---

Overall: Well-structured and faithful to Osherove 3rd Ed. The core concepts (entry/exit points, AAA, trust boundaries, anti-patterns) are accurately captured and the examples are idiomatic TypeScript/Vitest. Two issues to address before merging.

---

Issue 1 — Missing YAML Frontmatter (needs fix)

Every other SKILL.md in the repo includes a YAML frontmatter block used for skill discovery and registration. This file omits it entirely.

Compare with refactoring/SKILL.md lines 1–4:

---
name: refactoring
description: Inspect, review, and refactor code using patterns from ...
---

This file should have:

---
name: unit-testing
description: Write and review unit tests following Roy Osherove's "The Art of Unit Testing" (3rd Edition) — entry/exit points, trust boundaries, AAA, stubs/mocks/fakes, and anti-patterns.
---

Fix this →

---

Issue 2 — Mock masquerading as Fake in "For external services" example (line 156)

The file carefully defines the distinction between mocks and fakes:

Fake — Working implementation (in-memory), not an assertion target
Mock — Records calls for verification, is an assertion target

But the example at line 156 uses vi.fn() — which is a mock primitive — and names it fakeNotifier:

// line 156 — this is actually a mock, not a fake
const fakeNotifier = { notify: vi.fn().mockResolvedValue(undefined) };

If the intent is a fake (no assertion), it should be a plain object:

const fakeNotifier = { notify: async (_ids: number[], _text: string) => {} };

If the intent is to verify the call was made (a mock), rename it and assert:

const mockNotifier = { notify: vi.fn().mockResolvedValue(undefined) };
// ...
expect(mockNotifier.notify).toHaveBeenCalledWith(...);

Using a mock primitive in a "fake" example directly contradicts the skill's own guidance and will confuse readers.

Fix this →

---

Minor Notes (non-blocking)

• Line 74: "One logical assertion per test" is a good rule, but the parenthetical "(multiple expect calls OK if verifying one exit point)" could use a brief example — readers often misread this as license for unrelated multi-asserts.
• Lines 44–49: The it(...) calls inside describe have no function bodies, which is valid as pending tests in Vitest, but could look incomplete to readers unfamiliar with that pattern. A one-line comment like // shown as pending would clarify intent.
• Scope: The skill is TypeScript/Vitest-specific in all examples. If this repo targets polyglot teams, a note clarifying the language scope would help. (Not a blocker.)

---

Verdict: Fix the two issues above (frontmatter and mock/fake mislabeling) and this is ready to merge. The content itself is solid.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0ab2d338fb

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Add required YAML frontmatter at file start

This skill omits the --- YAML frontmatter block (name, description, etc.) that the rest of this repo’s skills use and that power-builder/SKILL.md documents as required for SKILL metadata; starting directly with a markdown heading makes this file inconsistent with the expected skill format and can cause loaders that parse frontmatter for discovery/display to skip or misclassify the new skill.

Useful? React with 👍 / 👎.