test: Increase test coverage for agent-core and composeApp#116
test: Increase test coverage for agent-core and composeApp#116yacosta738 merged 2 commits intomainfrom
Conversation
- Add unit tests for Greeting and ChatComponents utility functions in composeApp. - Add unit tests for ChatWorkspaceDefaults state initialization. - Enhance RustCliBridgeTest in agent-core-kmp with edge cases: invalid timeout, working directory support, and blank failure output. - Verified all new and existing tests pass using Gradle. Co-authored-by: yacosta738 <33158051+yacosta738@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Deploying corvus with
|
| Latest commit: |
54a8d30
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c8bbde86.corvus-42x.pages.dev |
| Branch Preview URL: | https://test-increase-coverage-10574.corvus-42x.pages.dev |
📝 WalkthroughWalkthroughThis PR introduces new test files for Greeting, ChatComponents, and ChatWorkspaceDefaults functionality, and extends RustCliBridgeTest with three additional test methods and an updated existing test case. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
✅ Contributor ReportUser: @yacosta738
Contributor Report evaluates based on public GitHub activity. Analysis period: 2025-03-01 to 2026-03-01 |
yacosta738
changed the title
|
![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
🧹 Nitpick comments (1)
modules/agent-core-kmp/src/jvmTest/kotlin/com/profiletailors/agent/core/RustCliBridgeTest.kt (1)
35-35: Avoid hardcoded/usr/bin/envin test commands.Using an absolute env path adds unnecessary environment coupling; the shell is already invoked, so direct exits are enough.
Proposed fix
- arguments = listOf("-c", "echo bridge-error >&2; /usr/bin/env sh -c 'exit 7'", "bridge"), + arguments = listOf("-c", "echo bridge-error >&2; exit 7", "bridge"), ... - arguments = listOf("-c", "/usr/bin/env sh -c 'exit 9'", "bridge"), + arguments = listOf("-c", "exit 9", "bridge"),Also applies to: 115-115
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/agent-core-kmp/src/jvmTest/kotlin/com/profiletailors/agent/core/RustCliBridgeTest.kt` at line 35, Test commands in RustCliBridgeTest use a hardcoded "/usr/bin/env" path; update the arguments list in the test(s) so they do not reference an absolute env path. Replace occurrences of "/usr/bin/env sh -c 'exit 7'" in the RustCliBridgeTest arguments with a portable form such as "sh -c 'exit 7'" (or simply "exit 7" if the invoked shell handles it) in both places where arguments = listOf(...) appears so the tests no longer depend on a hardcoded env path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@clients/composeApp/src/commonTest/kotlin/com/profiletailors/corvus/GreetingTest.kt`:
- Line 12: The assertion in GreetingTest.kt uses greeting.contains("Hello") but
its message says "Greeting should start with Hello"; update the test to make
intent and message consistent by using greeting.startsWith("Hello") in the
assertTrue (or alternatively adjust the message to say "should contain Hello" if
you intend to allow contains) so that the assertion and message match for the
assertion in the test function where greeting is validated.
In
`@modules/agent-core-kmp/src/jvmTest/kotlin/com/profiletailors/agent/core/RustCliBridgeTest.kt`:
- Around line 102-105: The test compares paths using expected =
File(tempDir).absolutePath.removeSuffix("/") and actual =
File(success.output.text).absolutePath.removeSuffix("/"), but
success.output.text may include a trailing newline; update the actual (and
optionally expected) construction to trim whitespace before converting to File
(e.g., call .trim() on success.output.text) so the comparison uses
File(success.output.text.trim()).absolutePath.removeSuffix("/"); locate these
variables in RustCliBridgeTest.kt (expected, actual, success.output.text) and
apply the trim to avoid flakiness.
---
Nitpick comments:
In
`@modules/agent-core-kmp/src/jvmTest/kotlin/com/profiletailors/agent/core/RustCliBridgeTest.kt`:
- Line 35: Test commands in RustCliBridgeTest use a hardcoded "/usr/bin/env"
path; update the arguments list in the test(s) so they do not reference an
absolute env path. Replace occurrences of "/usr/bin/env sh -c 'exit 7'" in the
RustCliBridgeTest arguments with a portable form such as "sh -c 'exit 7'" (or
simply "exit 7" if the invoked shell handles it) in both places where arguments
= listOf(...) appears so the tests no longer depend on a hardcoded env path.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
clients/composeApp/src/commonTest/kotlin/com/profiletailors/corvus/GreetingTest.ktclients/composeApp/src/commonTest/kotlin/com/profiletailors/corvus/ui/chat/ChatComponentsTest.ktclients/composeApp/src/commonTest/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspaceDefaultsTest.ktmodules/agent-core-kmp/src/jvmTest/kotlin/com/profiletailors/agent/core/RustCliBridgeTest.kt
| val platform = getPlatform() | ||
| val greeting = Greeting().greet() | ||
|
|
||
| assertTrue(greeting.contains("Hello"), "Greeting should start with Hello") |
There was a problem hiding this comment.
Assertion intent and message don’t match.
The check uses contains("Hello") while the message says the greeting should start with Hello, so this can pass for incorrect output.
Proposed fix
- assertTrue(greeting.contains("Hello"), "Greeting should start with Hello")
+ assertTrue(greeting.startsWith("Hello"), "Greeting should start with Hello")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| assertTrue(greeting.contains("Hello"), "Greeting should start with Hello") | |
| assertTrue(greeting.startsWith("Hello"), "Greeting should start with Hello") |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@clients/composeApp/src/commonTest/kotlin/com/profiletailors/corvus/GreetingTest.kt`
at line 12, The assertion in GreetingTest.kt uses greeting.contains("Hello") but
its message says "Greeting should start with Hello"; update the test to make
intent and message consistent by using greeting.startsWith("Hello") in the
assertTrue (or alternatively adjust the message to say "should contain Hello" if
you intend to allow contains) so that the assertion and message match for the
assertion in the test function where greeting is validated.
| // Normalize paths for comparison | ||
| val expected = File(tempDir).absolutePath.removeSuffix("/") | ||
| val actual = File(success.output.text).absolutePath.removeSuffix("/") | ||
| assertEquals(expected, actual) |
There was a problem hiding this comment.
Trim command output before path comparison.
pwd commonly emits a trailing newline; comparing the raw captured text can make this test flaky.
Proposed fix
- val actual = File(success.output.text).absolutePath.removeSuffix("/")
+ val actual = File(success.output.text.trim()).absolutePath.removeSuffix("/")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Normalize paths for comparison | |
| val expected = File(tempDir).absolutePath.removeSuffix("/") | |
| val actual = File(success.output.text).absolutePath.removeSuffix("/") | |
| assertEquals(expected, actual) | |
| // Normalize paths for comparison | |
| val expected = File(tempDir).absolutePath.removeSuffix("/") | |
| val actual = File(success.output.text.trim()).absolutePath.removeSuffix("/") | |
| assertEquals(expected, actual) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@modules/agent-core-kmp/src/jvmTest/kotlin/com/profiletailors/agent/core/RustCliBridgeTest.kt`
around lines 102 - 105, The test compares paths using expected =
File(tempDir).absolutePath.removeSuffix("/") and actual =
File(success.output.text).absolutePath.removeSuffix("/"), but
success.output.text may include a trailing newline; update the actual (and
optionally expected) construction to trim whitespace before converting to File
(e.g., call .trim() on success.output.text) so the comparison uses
File(success.output.text.trim()).absolutePath.removeSuffix("/"); locate these
variables in RustCliBridgeTest.kt (expected, actual, success.output.text) and
apply the trim to avoid flakiness.
1 participant
This PR increases the test coverage for the agent-core-kmp and composeApp modules by adding several unit tests for previously untested or under-tested components.
Key changes:
All changes follow the project's TDD approach and have been verified locally using ./gradlew check and ./gradlew :composeApp:jvmTest.
PR created automatically by Jules for task 10574148230699322893 started by @yacosta738
Summary by CodeRabbit
Release Notes