Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions app/src/main/java/com/coderabbit/app/calculator/Car.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package com.coderabbit.app.calculator

class Car {
fun start(isElectric: Boolean): Boolean {
return if (isElectric) {
return true
Comment on lines +5 to +6

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Remove redundant return statement.

Line 6 contains a redundant return statement since the if expression already returns a value on line 5.

-        return if (isElectric) {
-            return true
+        return if (isElectric) {
+            true
📝 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.

Suggested change
return if (isElectric) {
return true
return if (isElectric) {
true
🤖 Prompt for AI Agents
In app/src/main/java/com/coderabbit/app/calculator/Car.kt around lines 5 to 6,
remove the redundant return statement on line 6 inside the if expression. Since
the if expression itself returns a value, just use the expression without the
extra return keyword to fix the redundancy.

} else {
throw Exception()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Provide meaningful exception message.

The exception is thrown without a descriptive message, making debugging difficult. Consider providing context about why the car failed to start.

-            throw Exception()
+            throw Exception("Non-electric car failed to start")
📝 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.

Suggested change
throw Exception()
throw Exception("Non-electric car failed to start")
🧰 Tools
🪛 detekt (1.23.8)

[warning] 8-8: A call to the default constructor of an exception was detected. Instead one of the constructor overloads should be called. This allows to provide more meaningful exceptions.

(detekt.exceptions.ThrowingExceptionsWithoutMessageOrCause)

🤖 Prompt for AI Agents
In app/src/main/java/com/coderabbit/app/calculator/Car.kt at line 8, the thrown
Exception lacks a descriptive message. Update the throw statement to include a
meaningful message that explains why the car failed to start, improving clarity
and aiding debugging.

🛠️ Refactor suggestion

Use a specific exception with a meaningful message.

Throwing a generic Exception without a message makes debugging difficult. Consider using a more specific exception type with a descriptive message.

-throw Exception()
+throw IllegalStateException("Cannot start non-electric car")
🧰 Tools
🪛 detekt (1.23.8)

[warning] 8-8: A call to the default constructor of an exception was detected. Instead one of the constructor overloads should be called. This allows to provide more meaningful exceptions.

(detekt.exceptions.ThrowingExceptionsWithoutMessageOrCause)

🤖 Prompt for AI Agents
In app/src/main/java/com/coderabbit/app/calculator/Car.kt at line 8, replace the
generic Exception being thrown with a more specific exception type relevant to
the error context, and include a descriptive message explaining the cause of the
exception to improve debugging clarity.

⚠️ Potential issue

Provide meaningful exception message.

The static analysis tool correctly identified that throwing a generic Exception() without a message makes debugging difficult.

Apply this fix to address the detekt warning:

-            throw Exception()
+            throw Exception("Non-electric cars cannot be started")

Or consider using a more specific exception type:

-            throw Exception()
+            throw UnsupportedOperationException("Non-electric cars are not supported")
🧰 Tools
🪛 detekt (1.23.8)

[warning] 8-8: A call to the default constructor of an exception was detected. Instead one of the constructor overloads should be called. This allows to provide more meaningful exceptions.

(detekt.exceptions.ThrowingExceptionsWithoutMessageOrCause)

🤖 Prompt for AI Agents
In app/src/main/java/com/coderabbit/app/calculator/Car.kt at line 8, the code
throws a generic Exception without a message, which hinders debugging. Update
the throw statement to include a meaningful exception message that clearly
describes the error condition. Alternatively, replace the generic Exception with
a more specific exception type that better represents the error scenario.

}
}
Comment on lines +4 to +10

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Simplify method logic and improve business logic.

The current implementation has several issues:

  1. Redundant return statement on line 6 - the outer return is unnecessary
  2. Questionable business logic - why would a non-electric car fail to start?
  3. The method can be simplified

Consider this improved implementation:

-    fun start(isElectric: Boolean): Boolean {
-        return if (isElectric) {
-            return true
-        } else {
-            throw Exception()
-        }
-    }
+    fun start(isElectric: Boolean): Boolean {
+        return if (isElectric) {
+            true  // Electric car starts silently
+        } else {
+            // Non-electric cars can start too, but maybe make noise?
+            println("Engine starting...")
+            true
+        }
+    }

Or if throwing an exception is intentional, at least provide a meaningful message (addressing the detekt warning).

📝 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.

Suggested change
fun start(isElectric: Boolean): Boolean {
return if (isElectric) {
return true
} else {
throw Exception()
}
}
fun start(isElectric: Boolean): Boolean {
return if (isElectric) {
true // Electric car starts silently
} else {
// Non-electric cars can start too, but maybe make noise?
println("Engine starting...")
true
}
}
🧰 Tools
🪛 detekt (1.23.8)

[warning] 8-8: A call to the default constructor of an exception was detected. Instead one of the constructor overloads should be called. This allows to provide more meaningful exceptions.

(detekt.exceptions.ThrowingExceptionsWithoutMessageOrCause)

🤖 Prompt for AI Agents
In app/src/main/java/com/coderabbit/app/calculator/Car.kt lines 4 to 10, remove
the redundant inner return statement inside the if block, simplify the method by
returning true directly for electric cars, and revise the business logic to
handle non-electric cars more appropriately instead of throwing a generic
exception. If throwing an exception is necessary, include a meaningful message
to clarify the failure reason and satisfy static analysis tools.

}
13 changes: 13 additions & 0 deletions app/src/test/java/com/coderabbit/app/calculator/CarTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package com.coderabbit.app.calculator

import junit.framework.TestCase.assertTrue
import org.junit.Test

class CarTest {

@Test
fun testCar() {
Comment on lines +8 to +9

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Test function violates coding guidelines and naming conventions.

According to the coding guidelines and retrieved learnings, test functions should:

  1. Use @CustomTest annotation (currently missing)
  2. Follow naming convention "given (optional) ... when (required) ... then (required)"
-    @Test
-    fun testCar() {
+    @CustomTest
+    @Test
+    fun `when car is electric then start returns true`() {
📝 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.

Suggested change
@Test
fun testCar() {
@CustomTest
@Test
fun `when car is electric then start returns true`() {
🤖 Prompt for AI Agents
In app/src/test/java/com/coderabbit/app/calculator/CarTest.kt at lines 8 to 9,
the test function uses the standard @Test annotation and a generic name
"testCar" which violates coding guidelines. Replace the @Test annotation with
@CustomTest and rename the function to follow the "given ... when ... then ..."
naming convention, ensuring the "when" and "then" parts are included to clearly
describe the test scenario and expected outcome.

val car = Car()
assertTrue(car.start(true))
}
Comment on lines +6 to +12

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add test coverage for exception scenario.

The test only covers the success case but doesn't test the exception throwing behavior for non-electric cars, leaving the implementation partially untested.

Add a test for the exception case:

@CustomTest
@Test
fun `when car is not electric then start throws exception`() {
    val car = Car()
    assertThrows(Exception::class.java) {
        car.start(false)
    }
}

Don't forget to import org.junit.Assert.assertThrows.

🤖 Prompt for AI Agents
In app/src/test/java/com/coderabbit/app/calculator/CarTest.kt around lines 6 to
12, the existing test only covers the success scenario for starting an electric
car but lacks coverage for the exception thrown when starting a non-electric
car. Add a new test method annotated with @Test that creates a Car instance and
asserts that calling start(false) throws an Exception using assertThrows. Also,
import org.junit.Assert.assertThrows to enable this assertion.

Comment on lines +8 to +12

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix test naming convention and use @customTest annotation.

Based on the coding guidelines and retrieved learnings for this repository, test function names should follow the "given (optional) ... when (required) ... then (required)" naming convention, and test functions should use the @customTest annotation.

 @Test
+@CustomTest
-fun testCar() {
+fun `given electric car when start is called then returns true`() {
     val car = Car()
     assertTrue(car.start(true))
 }
🤖 Prompt for AI Agents
In app/src/test/java/com/coderabbit/app/calculator/CarTest.kt around lines 8 to
12, the test function name does not follow the required "given ... when ... then
..." naming convention and is missing the @CustomTest annotation. Rename the
test function to follow this convention, clearly describing the scenario,
action, and expected outcome, and replace the @Test annotation with @CustomTest
as per the repository guidelines.

⚠️ Potential issue

Follow coding guidelines for test naming and annotations.

The test method violates two coding guidelines:

  1. Test function names should follow "given (optional) ... when (required) ... then (required)" pattern
  2. Test functions should use @CustomTest annotation instead of @Test

Apply this diff to align with project guidelines:

-    @Test
-    fun testCar() {
+    @CustomTest
+    fun `when car is electric then start returns true`() {
         val car = Car()
         assertTrue(car.start(true))
     }
🤖 Prompt for AI Agents
In app/src/test/java/com/coderabbit/app/calculator/CarTest.kt around lines 8 to
12, rename the test function to follow the "given ... when ... then ..." naming
pattern, for example, "givenCar_whenStart_thenReturnsTrue". Also, replace the
@Test annotation with the @CustomTest annotation as required by the project
guidelines.

}
Comment on lines +6 to +13

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add test coverage for non-electric car scenario.

The current test only covers the happy path (electric car). Consider adding a test for the exception case when the car is not electric to ensure complete coverage.

@Test
@CustomTest
fun `given non-electric car when start is called then throws exception`() {
    val car = Car()
    assertThrows(IllegalStateException::class.java) {
        car.start(false)
    }
}
🤖 Prompt for AI Agents
In app/src/test/java/com/coderabbit/app/calculator/CarTest.kt around lines 6 to
13, the existing test only covers starting an electric car. Add a new test
method to cover the non-electric car scenario where calling start(false) should
throw an IllegalStateException. Implement this by creating a test annotated with
@Test and @CustomTest that asserts the exception is thrown when start(false) is
called on a Car instance.

🛠️ Refactor suggestion

Add test coverage for exception scenario.

The current test only covers the happy path (electric car). Consider adding a test for the exception case to ensure comprehensive coverage.

Add this test method to cover the exception scenario:

+    @CustomTest
+    fun `when car is not electric then start throws exception`() {
+        val car = Car()
+        assertThrows(Exception::class.java) {
+            car.start(false)
+        }
+    }
+

Don't forget to import assertThrows:

 import junit.framework.TestCase.assertTrue
 import org.junit.Test
+import org.junit.jupiter.api.assertThrows

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In app/src/test/java/com/coderabbit/app/calculator/CarTest.kt around lines 6 to
13, the current test only covers the successful start of the car. Add a new test
method that uses assertThrows to verify that the expected exception is thrown in
the failure scenario. Import assertThrows from the appropriate testing library
and write the test to call car.start with parameters that trigger the exception,
ensuring full test coverage.

3 changes: 3 additions & 0 deletions app/src/test/java/com/coderabbit/app/calculator/CustomTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package com.coderabbit.app.calculator

annotation class CustomTest()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

LGTM! Ensure the annotation is used according to coding guidelines.

The CustomTest annotation is correctly defined. However, according to the coding guidelines, test functions should be implemented with the @CustomTest annotation, but it's not being used in CarTest.kt.

Verify that this annotation is used consistently across test files:


🏁 Script executed:

#!/bin/bash
# Description: Check if @CustomTest annotation is being used in test files
# Expected: Test methods should use @CustomTest annotation per guidelines

# Search for test methods and check annotation usage
rg -A 2 -B 2 "@Test|@CustomTest" app/src/test/

Length of output: 6046


Apply @customTest Across All Test Methods

The CustomTest annotation is defined but isn’t used in any test class. Per .github/instructions.md, all test functions must be annotated with @CustomTest (not @Test) and named using the “given…when…then” convention.

Please update the following files:

  • app/src/test/java/com/coderabbit/app/ExampleUnitTest.java
  • app/src/test/java/com/coderabbit/app/calculator/CalculatorTest.kt
  • app/src/test/java/com/coderabbit/app/calculator/CarTest.kt

For each test method, replace:

@Test
public void testName() { … }

with:

@CustomTest
fun `given … when … then …` { … }
🤖 Prompt for AI Agents
In app/src/test/java/com/coderabbit/app/calculator/CustomTest.kt at line 3, the
CustomTest annotation is defined but not applied. You need to update all test
methods in the specified test files (ExampleUnitTest.java, CalculatorTest.kt,
CarTest.kt) by replacing the @Test annotation with @CustomTest and renaming each
test method to follow the "given…when…then" naming convention. This involves
changing method signatures from Java style to Kotlin style functions with
backtick-enclosed descriptive names.

169 changes: 0 additions & 169 deletions coderabbit.yaml

This file was deleted.