Car class added and Tests also added - #17
Conversation
WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant Test as CarTest
participant Car as Car
Test->>Car: startEngine()
Car-->>Test: true
Test->>Car: isEngineStarted()
Car-->>Test: true
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
app/src/main/java/com/coderabbit/app/calculator/Car.kt (2)
5-8: Avoid println in app codePrefer removing logging here or using a proper logger if truly needed (e.g., android.util.Log). println makes unit tests noisy and is discouraged in production paths.
1-1: Package/domain mismatchCar under calculator looks misplaced. Consider moving to a domain-aligned package (e.g., com.coderabbit.app.vehicle or .domain.car). This improves cohesion and discoverability.
Please confirm intended package structure; I can provide a follow-up refactor plan once confirmed.
app/src/test/java/com/coderabbit/app/calculator/CarTest.kt (1)
6-15: Add a complementary test for initial stateOnce Car tracks state, add a test for the default engine state to strengthen coverage and behavior clarity.
Example (adjust import/package as needed and annotate with @customTest):
@CustomTest fun givenNewCar_whenQueryIsEngineStarted_thenFalse() { val car = Car() assertTrue(!car.isEngineStarted()) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/src/main/java/com/coderabbit/app/calculator/Car.kt(1 hunks)app/src/test/java/com/coderabbit/app/calculator/CarTest.kt(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/test/**
⚙️ CodeRabbit Configuration File
**/test/**: - test function names should follow naming rules like given (optional) ... when (required) ... then(required)
- test function should be implemented @customTest annotation
Files:
app/src/test/java/com/coderabbit/app/calculator/CarTest.kt
🧠 Learnings (2)
📓 Common learnings
Learnt from: NamigGadir
PR: NamigGadir/CodeRabbitAndroid#0
File: :0-0
Timestamp: 2025-07-29T05:02:55.415Z
Learning: In the NamigGadir/CodeRabbitAndroid repository, test function names should follow the naming convention "given (optional) ... when (required) ... then (required)" as specified in the instructions.md file.
📚 Learning: 2025-07-29T05:02:55.415Z
Learnt from: NamigGadir
PR: NamigGadir/CodeRabbitAndroid#0
File: :0-0
Timestamp: 2025-07-29T05:02:55.415Z
Learning: In the NamigGadir/CodeRabbitAndroid repository, test function names should follow the naming convention "given (optional) ... when (required) ... then (required)" as specified in the instructions.md file.
Applied to files:
app/src/test/java/com/coderabbit/app/calculator/CarTest.kt
🔇 Additional comments (1)
app/src/test/java/com/coderabbit/app/calculator/CarTest.kt (1)
8-15: Ensure test annotation and naming follow guidelinesThe test method in CarTest.kt is neither annotated with
@CustomTestnor named according to the “given…when…then…” convention, so it will be skipped by the runner. I was unable to locate theCustomTestannotation in the codebase—please verify its package (or add it if missing) and then update your test as shown:--- a/app/src/test/java/com/coderabbit/app/calculator/CarTest.kt +++ b/app/src/test/java/com/coderabbit/app/calculator/CarTest.kt @@ class CarTest { - fun startEngine() { + @CustomTest + fun givenNewCar_whenStartEngine_thenReturnsTrue() { // Arrange val car = Car() // Act val result = car.startEngine() // Assert assertTrue(result) + // Optional: also verify internal state + assertTrue(car.isEngineStarted()) } }• Verify that
CustomTestis defined (e.g. incom.coderabbit.test) or adjust the import accordingly
• Rename the method to follow the repository’s “given…when…then…” rule and add@CustomTestso the test runner picks it up
| class Car { | ||
|
|
||
| fun startEngine(): Boolean { | ||
| println("Engine is starting...") | ||
| return true | ||
| } | ||
|
|
||
| fun isEngineStarted(): Boolean { | ||
| // Simulate engine status | ||
| return true | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make Car stateful and eliminate println side-effects
Both methods currently hardcode success and provide no observable state, which limits usefulness and testability. Model engine state and remove println side-effects.
Apply:
-class Car {
-
- fun startEngine(): Boolean {
- println("Engine is starting...")
- return true
- }
-
- fun isEngineStarted(): Boolean {
- // Simulate engine status
- return true
- }
-}
+class Car {
+ private var engineStarted: Boolean = false
+
+ fun startEngine(): Boolean {
+ if (engineStarted) return true // idempotent
+ // TODO: plug real start logic here
+ engineStarted = true
+ return true
+ }
+
+ fun isEngineStarted(): Boolean = engineStarted
+}🤖 Prompt for AI Agents
In app/src/main/java/com/coderabbit/app/calculator/Car.kt from lines 3 to 14,
the Car class methods startEngine and isEngineStarted always return true and do
not maintain any engine state, plus startEngine uses println causing side
effects. Refactor the class to include a private Boolean property to track the
engine state, update this state in startEngine without printing anything, and
have isEngineStarted return the current state instead of a hardcoded value.
|
@coderabbitai check out test names using default instructions |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 1082 🏁 Script executed: Length of output: 555 🏁 Script executed: Length of output: 5526 @NamigGadir
Here’s a compliant revision using the repo’s backtick style: package com.coderabbit.app.calculator
import org.junit.Test
import org.junit.Assert.assertTrue
class CarTest {
@Test
fun `given car when startEngine then returns true`() {
// Arrange
val car = Car()
// Act
val result = car.startEngine()
// Assert
assertTrue(result)
}
}Note: Several tests in CalculatorTest.kt also don’t follow the convention; happy to open a follow-up issue to track renaming across the suite if you’d like. 🧠 Learnings used |
Summary by CodeRabbit
New Features
Tests