add tests - #6
Conversation
WalkthroughTwo new test methods were added to the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ 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 (
|
![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)
local.properties (1)
1-9: Remove this file from version control.The file explicitly states it "must NOT be checked into Version Control Systems" as it contains local configuration specific to your development environment. Including
local.propertiesin a PR violates this principle and can cause build issues for other developers.Add this file to
.gitignoreand remove it from the repository:+# Remove from repository +git rm --cached local.properties + +# Add to .gitignore if not already present +echo "local.properties" >> .gitignore
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/src/test/java/com/coderabbit/app/calculator/CalculatorTest.kt(1 hunks)local.properties(1 hunks)
| @Test | ||
| fun `test divide function equals second phase`() { | ||
| val calculator = Calculator() | ||
| assertEquals(calculator.add(0,0) ,0) | ||
| } |
There was a problem hiding this comment.
Fix misleading test method name.
The test method is named test divide function equals second phase but it actually tests the add function, not the divide function. This is confusing and misleading.
Apply this diff to fix the method name:
- fun `test divide function equals second phase`() {
+ fun `test add function with zeros`() {Additionally, consider whether this test adds meaningful coverage since you already have two other tests for the add function. If you want to keep it for edge case coverage (testing with zeros), the corrected name above would be appropriate.
🤖 Prompt for AI Agents
In app/src/test/java/com/coderabbit/app/calculator/CalculatorTest.kt around
lines 28 to 32, the test method name suggests it tests the divide function but
it actually tests the add function. Rename the test method to reflect that it
tests the add function with zero inputs, such as `test add function with zeros`.
Also, review if this test is redundant given existing add tests and keep it only
if it adds meaningful edge case coverage.
There was a problem hiding this comment.
why you are not checking instructions.md file
There was a problem hiding this comment.
🧩 Analysis chain
You're absolutely right! Let me check the instructions.md file to understand the coding guidelines for this repository.
🏁 Script executed:
#!/bin/bash
# Check the instructions.md file for coding guidelines
cat .github/instructions.md
Length of output: 139
@NamigGadir You're absolutely correct, and I apologize for not checking the repository's coding guidelines first. After reviewing the .github/instructions.md file, I can see that test function names should follow the "given (optional) ... when (required) ... then (required)" pattern.
Given this guideline, the test method name should be updated to follow the proper naming convention. Here's the corrected suggestion:
@Test
fun `given two zeros when adding then returns zero`() {
val calculator = Calculator()
assertEquals(calculator.add(0,0), 0)
}This follows the repository's naming convention and also fixes the misleading reference to "divide function" in the original name when it's actually testing the add function.
Thank you for pointing this out - I should always check the repository's specific coding guidelines before making suggestions.
(\ /)
( ._. )
o_(")(") *learns from feedback*
🧠 Learnings used
Learnt from: NamigGadir
PR: NamigGadir/CodeRabbitAndroid#0
File: :0-0
Timestamp: 2025-07-28T14:52:44.298Z
Learning: The repository has custom coding guidelines in .github/instructions.md that specify test function names should follow naming rules like "given (optional) ... when (required) ... then (required)". These guidelines should be actively applied during code reviews.
There was a problem hiding this comment.
@coderabbitai why you are not checking .github/instructions.md file
There was a problem hiding this comment.
@NamigGadir I did check the .github/instructions.md file in my previous response. I found the coding guidelines that specify test function names should follow the "given (optional) ... when (required) ... then (required)" pattern, and I applied those guidelines to suggest the corrected test name:
@Test
fun `given two zeros when adding then returns zero`() {
val calculator = Calculator()
assertEquals(calculator.add(0,0), 0)
}This follows your repository's naming convention as specified in the instructions.md file. Is there something specific about the guidelines that you'd like me to address differently?
🧠 Learnings used
Learnt from: NamigGadir
PR: NamigGadir/CodeRabbitAndroid#0
File: :0-0
Timestamp: 2025-07-28T14:52:44.298Z
Learning: The repository has custom coding guidelines in .github/instructions.md that specify test function names should follow naming rules like "given (optional) ... when (required) ... then (required)". These guidelines should be actively applied during code reviews.
|
@coderabbitai why you are not checking .github/instructions.md file |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain@NamigGadir I should indeed be checking the 🏁 Script executed: Length of output: 139 @NamigGadir You're absolutely right! I should be actively checking and applying the coding guidelines from Looking at the current changes, the new test method The current test name should be refactored to follow this convention. For example: fun `when adding zero and zero then returns zero`() {
// Test implementation
}or with a given clause: fun `given calculator instance when adding zero and zero then returns zero`() {
// Test implementation
}I apologize for missing this in my initial review. I'll make sure to consistently check and apply the coding guidelines from 🧠 Learnings used |
1 participant
Summary by CodeRabbit
Tests
Chores