test: add unit tests for buildProfile style analysis - #262
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (1)tests/**/*.mjs📄 CodeRabbit inference engine (AGENTS.md)
Files:
🔇 Additional comments (1)
📝 WalkthroughWalkthroughThe test suite now validates ChangesProfile metric tests
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="tests/history-profile.test.mjs">
<violation number="1" location="tests/history-profile.test.mjs:199">
P3: The three new tests each repeat the same ~14-line environment setup/teardown block (saving HOME/APPDATA/XDG_CONFIG_HOME, creating a temp home, setting the three env vars, then restoring them and deleting the temp dir in `finally`). This boilerplate already appears three times earlier in the file, so it now exists six times. Consider extracting a small helper like `withTempHome(async (tempHome) => { ... })` that owns the env save/set/restore and `rmSync` cleanup, and have each test call `writeHistory` and `buildProfile` inside it. This keeps the new tests focused on the ratio logic and avoids six copies of the same fragile env/teardown code.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| }); | ||
|
|
||
| test('buildProfile computes scope-usage ratio', async () => { | ||
| const originalHome = process.env.HOME; |
There was a problem hiding this comment.
P3: The three new tests each repeat the same ~14-line environment setup/teardown block (saving HOME/APPDATA/XDG_CONFIG_HOME, creating a temp home, setting the three env vars, then restoring them and deleting the temp dir in finally). This boilerplate already appears three times earlier in the file, so it now exists six times. Consider extracting a small helper like withTempHome(async (tempHome) => { ... }) that owns the env save/set/restore and rmSync cleanup, and have each test call writeHistory and buildProfile inside it. This keeps the new tests focused on the ratio logic and avoids six copies of the same fragile env/teardown code.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/history-profile.test.mjs, line 199:
<comment>The three new tests each repeat the same ~14-line environment setup/teardown block (saving HOME/APPDATA/XDG_CONFIG_HOME, creating a temp home, setting the three env vars, then restoring them and deleting the temp dir in `finally`). This boilerplate already appears three times earlier in the file, so it now exists six times. Consider extracting a small helper like `withTempHome(async (tempHome) => { ... })` that owns the env save/set/restore and `rmSync` cleanup, and have each test call `writeHistory` and `buildProfile` inside it. This keeps the new tests focused on the ratio logic and avoids six copies of the same fragile env/teardown code.</comment>
<file context>
@@ -194,3 +194,88 @@ test('formatProfile renders dominant tone, capitalization, scope, body, and pref
});
+
+test('buildProfile computes scope-usage ratio', async () => {
+ const originalHome = process.env.HOME;
+ const originalAppData = process.env.APPDATA;
+ const originalXdgConfigHome = process.env.XDG_CONFIG_HOME;
</file context>
404-Page-Found
left a comment
404-Page-Found
left a comment
There was a problem hiding this comment.
These tests mutate process-global environment variables (HOME, APPDATA, and XDG_CONFIG_HOME) around an asynchronous buildProfile() call. That can make the suite flaky if tests run concurrently or in a shared process, since other tests may observe the temporary paths. Please avoid global environment mutation by injecting the history/config path, or explicitly serialize these tests and centralize the setup/cleanup.
2 participants
Closes #236. This adds unit tests covering the profile-building logic including scope-usage ratio, body-usage ratio, and the empty history edge case.
Summary by CodeRabbit