Add skill for writing unit tests for PatternFly ecosystem components#29
Conversation
|
|
||
| ## Output | ||
|
|
||
| Output the complete test file ready to save. Name it `ComponentName.test.tsx`. |
There was a problem hiding this comment.
The existing unit-test-generator covers the same task in 69 lines by trusting the AI to know RTL. Most of this (query priority, userEvent vs fireEvent, findBy vs waitFor) is standard RTL knowledge the AI already has — the unique value here is the PatternFly wiki conventions (test() vs it() nesting, mock child components, snapshot policy, toHaveClass for CSS). Could those stand alone at ~80-100 lines with the RTL reference material moved to a reference.md?
| name: pf-library-test-writer | ||
| description: Write unit tests for PatternFly ecosystem components following official testing standards | ||
| --- | ||
|
|
There was a problem hiding this comment.
How does someone know when to use this vs. unit-test-generator? A one-liner in the description like "for contributors to PatternFly libraries, not consumers" would help.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdded a new documented skill, Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Guidelines referenced
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/pf-react/README.md`:
- Around line 79-82: The unlabeled fenced code block containing the file-tree in
README.md should be updated to include a language identifier (e.g., use ```text)
so markdownlint rule MD040 is satisfied; locate the file-tree block in the
pf-react README.md (the block showing the pf-react/ directory structure) and
change the opening fence from ``` to ```text (or another appropriate language)
and save.
In `@plugins/pf-react/skills/pf-library-test-writer/SKILL.md`:
- Around line 32-38: Add a language tag to the fenced code block that lists the
Button files so markdownlint MD040 is satisfied; update the block that starts
with "Button/" to use a language identifier such as ```text (e.g., replace ```
with ```text) so the block remains the same content but has a language
specified.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 18a57271-32bf-4d08-9ac9-5453ffc937c1
📒 Files selected for processing (2)
plugins/pf-react/README.mdplugins/pf-react/skills/pf-library-test-writer/SKILL.md
plugins/pf-react/README.md
Outdated
| │ ├── unit-test-generator/ | ||
| │ │ └── SKILL.md | ||
| │ └── pf-unit-test-generator/ | ||
| │ └── pf-library-test-writer/ | ||
| │ └── SKILL.md |
There was a problem hiding this comment.
Add a language identifier to the file-tree fenced block (MD040).
The file-structure code fence (opened at Line 69) is unlabeled, which triggers markdownlint.
Suggested fix
-```
+```text
pf-react/
├── .claude-plugin/
│ └── plugin.json # Plugin manifest + MCP server config
...
└── README.md</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @plugins/pf-react/README.md around lines 79 - 82, The unlabeled fenced code
block containing the file-tree in README.md should be updated to include a
language identifier (e.g., use text) so markdownlint rule MD040 is satisfied; locate the file-tree block in the pf-react README.md (the block showing the pf-react/ directory structure) and change the opening fence from to ```text
(or another appropriate language) and save.
</details>
<!-- fingerprinting:phantom:triton:hawk:7d06e245-2cfa-412e-90e5-d1f571512576 -->
<!-- This is an auto-generated comment by CodeRabbit -->
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
♻️ Duplicate comments (2)
plugins/pf-react/skills/pf-library-test-writer/SKILL.md (1)
32-38:⚠️ Potential issue | 🟡 MinorAdd language identifier to fenced code block (MD040).
The fenced code block showing the file tree is missing a language tag, triggering markdownlint MD040.
🔧 Proposed fix
-``` +```text Button/ ├── Button.tsx ├── Button.test.tsx ├── ButtonVariant.tsx └── ButtonVariant.test.tsx</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@plugins/pf-react/skills/pf-library-test-writer/SKILL.mdaround lines 32 -
38, The fenced code block showing the file tree lacks a language identifier;
update the triple-backtick fence surrounding the block that contains "Button/
├── Button.tsx ..." to include a language tag (e.g., ```text) so the code block
is valid for markdownlint MD040; keep the block content unchanged and only
modify the opening fence to add the language identifier.</details> </blockquote></details> <details> <summary>plugins/pf-react/README.md (1)</summary><blockquote> `70-70`: _⚠️ Potential issue_ | _🟡 Minor_ **Add language identifier to file tree fenced block (MD040).** The file structure code fence is missing a language tag, triggering markdownlint MD040. <details> <summary>🔧 Proposed fix</summary> ```diff -``` +```text pf-react/ ├── .claude-plugin/ │ └── plugin.json ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@plugins/pf-react/README.mdat line 70, The README's file-tree fenced code
block is missing a language identifier which triggers markdownlint MD040; update
the fenced block in the README (the file-tree block that starts with triple
backticks) to include a language tag such as "text" (e.g., change ``` toresolved.
🧹 Nitpick comments (1)
plugins/pf-react/skills/pf-library-test-writer/SKILL.md (1)
6-102: Consider separating RTL reference material intoreference.mdper Anthropic skill-creator guidelines.Anthropic's skill-creator plugin (referenced in CONTRIBUTING-SKILLS.md) recommends progressive disclosure: keep
SKILL.mdunder 500 lines as an overview/navigation entry point, and move detailed reference material into separatereferences/files. This skill's 102 lines blend PatternFly-specific conventions (mock child components,test()vsit()nesting, snapshot policy,toHaveClassfor CSS) with substantial React Testing Library reference material (query priorities,fireEventvsuserEvent,findByvswaitFor).The sibling skill
pf-bug-triage(110 lines) follows this pattern with a companionreference.md, as do other skills in the repo (pf-token-auditor,analytics-repo-pruning). Moving general RTL best practices to areference.mdwould sharpenSKILL.mdto focus exclusively on PatternFly-specific requirements and your contribution's unique value, while keeping RTL fundamentals available for developers who need them.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/pf-react/skills/pf-library-test-writer/SKILL.md` around lines 6 - 102, SKILL.md currently mixes PatternFly-specific testing rules with broad React Testing Library (RTL) reference material; split the content by creating a new references/reference.md (or references/RTL-reference.md) and move general RTL topics (query priorities, fireEvent vs userEvent, findBy vs waitFor, query examples, and extensive snapshot guidance) into that file, leaving SKILL.md as a concise PatternFly-focused overview under 500 lines that retains the coverage checklist, mocking guidance, and PatternFly-specific conventions; update SKILL.md to link to the new reference file and ensure headings/links mention pf-library-test-writer and the new reference so reviewers can find the detailed RTL guidance.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@plugins/pf-react/README.md`:
- Line 70: The README's file-tree fenced code block is missing a language
identifier which triggers markdownlint MD040; update the fenced block in the
README (the file-tree block that starts with triple backticks) to include a
language tag such as "text" (e.g., change ``` to ```text) so the code fence is
properly identified and the linter warning is resolved.
In `@plugins/pf-react/skills/pf-library-test-writer/SKILL.md`:
- Around line 32-38: The fenced code block showing the file tree lacks a
language identifier; update the triple-backtick fence surrounding the block that
contains "Button/ ├── Button.tsx ..." to include a language tag (e.g., ```text)
so the code block is valid for markdownlint MD040; keep the block content
unchanged and only modify the opening fence to add the language identifier.
---
Nitpick comments:
In `@plugins/pf-react/skills/pf-library-test-writer/SKILL.md`:
- Around line 6-102: SKILL.md currently mixes PatternFly-specific testing rules
with broad React Testing Library (RTL) reference material; split the content by
creating a new references/reference.md (or references/RTL-reference.md) and move
general RTL topics (query priorities, fireEvent vs userEvent, findBy vs waitFor,
query examples, and extensive snapshot guidance) into that file, leaving
SKILL.md as a concise PatternFly-focused overview under 500 lines that retains
the coverage checklist, mocking guidance, and PatternFly-specific conventions;
update SKILL.md to link to the new reference file and ensure headings/links
mention pf-library-test-writer and the new reference so reviewers can find the
detailed RTL guidance.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 017bc153-0fa5-4d06-91f5-b27d0994171e
📒 Files selected for processing (2)
plugins/pf-react/README.mdplugins/pf-react/skills/pf-library-test-writer/SKILL.md
| description: Write unit tests for contributors to PatternFly libraries (patternfly-react, patternfly-chatbot, etc.), not for consumers of PatternFly components. Use `unit-test-generator` for consumer application tests instead. | ||
| --- | ||
|
|
||
| Write unit tests for components and features within the PatternFly ecosystem (patternfly-react, patternfly-chatbot, patternfly-virtual-assistant, and other JS/TS-based PatternFly libraries). All tests must strictly follow the [official PatternFly testing guidelines](https://github.com/patternfly/patternfly-react/wiki/React-Testing-Library-Basics,-Best-Practices,-and-Guidelines). |
There was a problem hiding this comment.
"All tests must strictly follow" the wiki link creates a tension - if the wiki changes and diverges from the rules here, the AI has two conflicting sources of truth. Since the skill already captures the key rules inline, could this just reference the wiki as background context rather than the authoritative source?
| ## How to Write Tests | ||
|
|
||
| 1. Read the component source to understand its props, children, and behavior. | ||
| 2. If the PatternFly MCP server is available, use `usePatternFlyDocs` to check the component's props schema, variants, and accessibility requirements. |
There was a problem hiding this comment.
This is prescribing implementation - the AI already knows how to use available MCP tools. Per CONTRIBUTING-SKILLS.md: "Describe outcomes, not implementation."
|
|
||
| ## Mocking Child Components | ||
|
|
||
| Default to **mocking child components** for unit testing prop-passing behavior: |
There was a problem hiding this comment.
This is the key differentiator from pf-unit-test-generator (which says "never mock child components"), but it's buried mid-file. Since this is the main reason someone would pick one skill over the other, would it help to surface this more prominently - maybe in the opening paragraph?
| ## Test File Structure | ||
|
|
||
| ```typescript | ||
| import React from 'react'; |
There was a problem hiding this comment.
nit: import React from 'react' isn't needed with modern JSX transform (React 17+) and PF repos don't include it.
2 participants
Closes #22
Summary by CodeRabbit