tests added - #11
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughA new unit test was added to the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
app/src/test/java/com/coderabbit/app/calculator/CalculatorTest.kt (1)
31-31: Fix incorrect method call in divide test.This test is named "test divide function equals second phase" but calls
calculator.add(0,0)instead ofcalculator.divide(). This is likely a copy-paste error.Apply this diff to fix the method call:
- assertEquals(calculator.add(0,0) ,0) + assertEquals(calculator.divide(0,2) ,0)Note: I assumed dividing 0 by 2 as a reasonable test case. Adjust the divisor as needed based on your test requirements.
🧹 Nitpick comments (1)
app/src/test/java/com/coderabbit/app/calculator/CalculatorTest.kt (1)
50-56: Fix test naming and improve consistency.The test name contains a typo ("che" should be "that") and inconsistent naming patterns throughout the file.
Consider standardizing test names:
- fun `test divide function che divide forty`() { + fun `test divide function throws exception when dividing forty by zero`() {
| @Test | ||
| fun `test divide function che divide forty`() { | ||
| val calculator = Calculator() | ||
| assertThrows(ArithmeticException::class.java){ | ||
| calculator.divide(40,0) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider consolidating duplicate division by zero tests.
This test is functionally identical to existing division by zero tests (lines 35-40, 43-48, 60-65, 68-73, 76-81). All test the same behavior with different dividend values, which doesn't add meaningful test coverage.
Consider using a parameterized test to reduce duplication:
+import org.junit.runner.RunWith
+import org.junit.runners.Parameterized
+@RunWith(Parameterized::class)
+class CalculatorDivisionByZeroTest(private val dividend: Int) {
+
+ companion object {
+ @JvmStatic
+ @Parameterized.Parameters
+ fun data(): Collection<Array<Any>> {
+ return listOf(
+ arrayOf(4), arrayOf(13), arrayOf(40),
+ arrayOf(14), arrayOf(10), arrayOf(17)
+ )
+ }
+ }
+
+ @Test
+ fun `test divide by zero throws ArithmeticException`() {
+ val calculator = Calculator()
+ assertThrows(ArithmeticException::class.java) {
+ calculator.divide(dividend, 0)
+ }
+ }
+}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/src/test/java/com/coderabbit/app/calculator/CalculatorTest.kt around
lines 50 to 56, the test for division by zero duplicates existing tests with
different dividend values. Refactor by consolidating these into a single
parameterized test that iterates over multiple dividend values to check for
ArithmeticException when dividing by zero, reducing code duplication and
maintaining coverage.
ad7e8c0 to
c6c085c
Compare
1 participant
Summary by CodeRabbit