Tech/add new tests zeor - #5
Conversation
WalkthroughThree new unit tests were added to the CalculatorTest class to verify that the Calculator's divide function throws an ArithmeticException when attempting to divide by zero. The tests use assertThrows to check exception handling for inputs 4/0, 10/0, and 17/0. No changes were made to application logic or public interfaces. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 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. ✨ Finishing Touches
🧪 Generate unit tests
🪧 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 (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
app/src/test/java/com/coderabbit/app/calculator/CalculatorTest.kt (3)
28-34: Improve test method naming for clarity.The test name
test divide function che dividecontains unclear wording. Consider a more descriptive name that clearly indicates the test purpose.Apply this diff to improve the test name:
- fun `test divide function che divide`() { + fun `test divide function throws exception when dividing by zero`() {
36-42: Improve test method naming for clarity.The test name
test divide function che divide zerocontains unclear wording. Consider a more descriptive name that clearly indicates the test purpose.Apply this diff to improve the test name:
- fun `test divide function che divide zero`() { + fun `test divide function throws exception when dividing ten by zero`() {
28-50: Consider using parameterized tests to reduce duplication.The three divide-by-zero tests follow identical patterns with only the dividend changing. Consider refactoring to use parameterized tests for better maintainability.
Here's a parameterized test approach that would replace all three tests:
@ParameterizedTest @ValueSource(ints = [4, 10, 17]) fun `test divide function throws exception when dividing by zero`(dividend: Int) { val calculator = Calculator() assertThrows(ArithmeticException::class.java) { calculator.divide(dividend, 0) } }You would need to add this import:
import org.junit.jupiter.params.ParameterizedTest import org.junit.jupiter.params.provider.ValueSource
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/src/test/java/com/coderabbit/app/calculator/CalculatorTest.kt(2 hunks)
🔇 Additional comments (1)
app/src/test/java/com/coderabbit/app/calculator/CalculatorTest.kt (1)
4-4: LGTM!The assertThrows import is correctly added to support the new exception testing functionality.
| @Test | ||
| fun `test divide function che divide zero by 17`() { | ||
| val calculator = Calculator() | ||
| assertThrows(ArithmeticException::class.java){ | ||
| calculator.divide(17,0) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix test name logic and consider reducing duplication.
The test name test divide function che divide zero by 17 is backwards - the code divides 17 by 0, not zero by 17.
Apply this diff to fix the test name:
- fun `test divide function che divide zero by 17`() {
+ fun `test divide function throws exception when dividing seventeen by zero`() {🤖 Prompt for AI Agents
In app/src/test/java/com/coderabbit/app/calculator/CalculatorTest.kt around
lines 44 to 50, the test name incorrectly states "divide zero by 17" while the
code divides 17 by 0. Rename the test function to accurately reflect the
operation being tested, such as "test divide function divides 17 by zero". Also,
review the test class for similar test name inconsistencies and consider
extracting common setup code to reduce duplication.
|
@coderabbitai are you using my instructions.md file that i have provided for you? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 80 🏁 Script executed: Length of output: 931 @NamigGadir I searched thoroughly for an
The repository currently doesn't contain any markdown files or instruction files. If you intended to provide me with specific instructions through an
I'm currently following the standard CodeRabbit review practices, but I'd be happy to incorporate any specific instructions you provide! |
|
@coderabbitai i have added this file .github/instructions.md in your instructions but unfortunately you are not unsing this instructions while review. can you check your config and this file again? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 921 🛠️ Debug Information Gathered @NamigGadir, great news! I found your Your Instructions FileYour Configuration StatusLooking at my current configuration, I can confirm that: code_guidelines:
enabled: true
filePatterns:
- .github/instructions.mdThe IssueThe reason I wasn't applying your custom instructions in my previous reviews is that I need to actively incorporate these guidelines into my analysis. Your instruction about test function naming conventions should have been applied to the calculator test review. Moving ForwardI will now ensure that I:
For future reviews, I'll specifically check test method names against your naming convention and suggest improvements when they don't follow the Would you like me to review the current PR again with your custom instructions properly applied? ✏️ Learnings added
|
Summary by CodeRabbit