fix: prevent duplicate generate:after hook execution#2036
fix: prevent duplicate generate:after hook execution#2036SushanthMusham wants to merge 2 commits intoasyncapi:masterfrom
Conversation
SushanthMusham
requested review from
asyncapi-bot-eve,
derberg,
jonaslagoni and
magicmatatjahu
as code owners
|
What reviewer looks at during PR reviewThe following are ideal points maintainers look for during review. Reviewing these points yourself beforehand can help streamline the review process and reduce time to merge.
|
|
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 as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRemoved a duplicate invocation of Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 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: 1
🧹 Nitpick comments (1)
apps/generator/test/generator.test.js (1)
138-159: Consider extending the hook count assertion to other#generatetest cases.For comprehensive regression coverage, consider adding the
afterHookCalls.length === 1assertion to the other test cases that verifygenerate:afteris called (lines 174, 193, 213, 234, 255). This would ensure the "exactly once" invariant is consistently validated across all generation paths.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/generator/test/generator.test.js` around lines 138 - 159, The tests currently assert that gen.launchHook was called with 'generate:after' in one case; extend the same "exactly once" assertion to the other generate tests by adding the same check used here (compute afterHookCalls = gen.launchHook.mock.calls.filter(call => call[0] === 'generate:after') and assert afterHookCalls.length === 1) inside each test that exercises Generator.prototype.generate (i.e., the other it blocks that create a new Generator, call gen.generate, and already assert gen.launchHook). Ensure you reference the same mock function (gen.launchHook) and the same hook name ('generate:after') so each test verifies the hook is invoked exactly once.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/generator/test/generator.test.js`:
- Around line 152-153: Add the same assertion that checks generate:after was
called once to the "entrypoint=set" test case (the test that sets output='fs',
forceWrite=false, install=false, entrypoint='set'); locate the test where
gen.launchHook is mocked and add: const afterHookCalls =
gen.launchHook.mock.calls.filter(call => call[0] === 'generate:after');
expect(afterHookCalls.length).toBe(1); so the regression for entrypoint=set with
output='fs' is covered (refer to gen.launchHook and the entrypoint=set test).
---
Nitpick comments:
In `@apps/generator/test/generator.test.js`:
- Around line 138-159: The tests currently assert that gen.launchHook was called
with 'generate:after' in one case; extend the same "exactly once" assertion to
the other generate tests by adding the same check used here (compute
afterHookCalls = gen.launchHook.mock.calls.filter(call => call[0] ===
'generate:after') and assert afterHookCalls.length === 1) inside each test that
exercises Generator.prototype.generate (i.e., the other it blocks that create a
new Generator, call gen.generate, and already assert gen.launchHook). Ensure you
reference the same mock function (gen.launchHook) and the same hook name
('generate:after') so each test verifies the hook is invoked exactly once.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2a277ef6-ad99-4950-9cb3-e02327197887
📒 Files selected for processing (2)
apps/generator/lib/generator.jsapps/generator/test/generator.test.js
💤 Files with no reviewable changes (1)
- apps/generator/lib/generator.js
|
Please read the guidelines, donot raise pr without maintainers consent |
|
Adi-204
left a comment
Adi-204
left a comment
There was a problem hiding this comment.
@SushanthMusham the linting is failing you can run npm run lint to check whether it is passing or not.
4 participants
Description
generate:afterhook was being executed twice when using the Generator API withoutput: 'fs'and a specifiedentrypoint.launchHook('generate:after')call insidehandleEntrypointinapps/generator/lib/generator.jsto centralize hook execution and prevent duplicate side effects.apps/generator/test/generator.test.jsto explicitly verify that thegenerate:afterhook runs exactly once per generation lifecycle.Related issue(s)
Fixes #1993
Summary by CodeRabbit
Bug Fixes
Tests