chore: improve tests and fix typo

[ymc9]

Summary by CodeRabbit

• Chores
  • Streamlined plugin configuration handling to reduce dependencies.
  • Refined internal documentation formatting.

[claude]

Claude finished @ymc9's task —— View job

---

PR Review Results ✅

Reviewing PR #311: "chore: improve tests and fix typo"

Todo List:

• Read and analyze changed files
• Review code quality and best practices
• Check for potential bugs or issues
• Assess performance considerations
• Evaluate security concerns
• Review test coverage implications
• Provide comprehensive feedback

Changes Analysis:

File 1: packages/plugins/policy/plugin.zmodel:4

• Change: Fixed typo in JSDoc comment - removed extra space in " "delete" → "delete"
• Impact: Documentation clarity improvement

File 2: packages/testtools/src/utils.ts

• Change: Replaced manual path resolution with require.resolve()
• Removed: import path from 'node:path'; (line 2)
• Improved: path.resolve(__dirname, '../node_modules/...') → require.resolve('@zenstackhq/plugin-policy/plugin.zmodel')

Review Feedback:

✅ Code Quality & Best Practices

• Excellent improvement in utils.ts - require.resolve() is the correct Node.js pattern for resolving module paths
• The change removes hardcoded relative paths and makes the code more robust across different environments
• Proper cleanup of unused import (path)

✅ Potential Bugs/Issues

• Fixes a potential bug: The old approach using __dirname and relative paths could break in different build/deployment scenarios
• require.resolve() will correctly locate the module regardless of installation structure
• No new bugs introduced

✅ Performance Considerations

• Minor performance improvement: eliminates unnecessary path module import
• require.resolve() is efficient and cached by Node.js

✅ Security Concerns

• No security implications in these changes
• Actually improves security posture by using Node.js standard module resolution instead of manual path construction

✅ Test Coverage

• Changes don't affect test coverage directly
• The loadDocumentWithPlugins function behavior remains the same, just with more reliable path resolution
• No additional tests needed for these refactoring changes

Recommendation: APPROVE ✅

This is a clean, well-executed maintenance PR that:

1. Fixes a documentation typo
2. Improves code reliability using proper Node.js patterns
3. Removes technical debt (hardcoded paths)
4. Follows best practices

The changes are minimal, safe, and improve the codebase quality.

---

[coderabbitai]

Walkthrough

Two files are updated: a documentation comment correction in the policy plugin model removing a stray space before "delete", and a path resolution refactor in testtools that replaces path.resolve(__dirname, ...) with require.resolve('@zenstackhq/plugin-policy/plugin.zmodel'), eliminating the path module dependency.

Changes

Cohort / File(s)	Change Summary
Documentation Fix packages/plugins/policy/plugin.zmodel	Removed stray space before "delete" in operation documentation comment
Path Resolution Refactor packages/testtools/src/utils.ts	Replaced path module dependency and path.resolve(__dirname, ...) logic with require.resolve('@zenstackhq/plugin-policy/plugin.zmodel') to resolve plugin model file path

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• refactor: move policy zmodel back to stdlib #265 — Refactors/centralizes plugin zmodel location, which this change builds upon by updating the path resolution method
• refactor: move policy attributes to its own zmodel #307 — Introduces the exported require.resolve target that this PR now directly references for plugin path resolution

Poem

🐰 A space hopped away from the delete line,
And paths now resolve in style so fine,
No more __dirname, just require.resolve,
A tidy cleanup—a rabbit's absolve! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title "chore: improve tests and fix typo" is partially related to the changeset. It accurately references the typo fix in the documentation comment of the plugin.zmodel file. However, the phrase "improve tests" is somewhat vague regarding the actual changes in packages/testtools/src/utils.ts, where the main functional change involves refactoring path resolution logic from path.resolve(__dirname, ...) to require.resolve(...). While this refactoring could be considered an improvement to the test utilities, the title doesn't clearly convey this specific change, though it does refer to a real part of the changeset.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch chore/test-hardcode

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7ef504c and dbe922f.

📒 Files selected for processing (2)

• packages/plugins/policy/plugin.zmodel (1 hunks)
• packages/testtools/src/utils.ts (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

{packages,samples,tests}/**

📄 CodeRabbit inference engine (CLAUDE.md)

Place packages only under packages/, samples/, or tests/

Files:

• packages/testtools/src/utils.ts
• packages/plugins/policy/plugin.zmodel

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: build-test (20.x, sqlite)
• GitHub Check: build-test (20.x, postgresql)

🔇 Additional comments (2)

packages/plugins/policy/plugin.zmodel (1)

4-4: LGTM! Documentation formatting improved.

The stray space has been removed, making the operation list formatting consistent with other similar comments in the file.

packages/testtools/src/utils.ts (1)

3-6: Code is already committed and properly configured for module resolution.

The code currently uses require.resolve() to resolve the plugin module subpath. The package configuration correctly exports ./plugin.zmodel via the exports map, and the file is included in the published distribution. However, module resolution can only be fully validated in a proper development environment with packages installed and linked via pnpm workspaces—the sandbox cannot test this.

Ensure your CI pipeline runs the test suite (which imports and uses loadDocumentWithPlugins across multiple test files) to confirm module resolution works correctly in your build and deployment environments.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Pull Request Overview

This PR updates test tooling to resolve the plugin model path more robustly and fixes a typo in plugin documentation.

• Switch path resolution from a hardcoded node_modules path to module-based resolution.
• Correct a documentation typo in plugin.zmodel.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
packages/testtools/src/utils.ts	Replace path.resolve + __dirname with require.resolve for locating plugin.zmodel.
packages/plugins/policy/plugin.zmodel	Fix typo in the parameter description for the list of operations.

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}