-
Notifications
You must be signed in to change notification settings - Fork 0
Add Tests for PR#23 #30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: rohitvinnakota-codecov-patch-21
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -29,3 +29,9 @@ def test_divide(): | |||||||||||||||||||||||||||||||||||||||||||
| assert Calculator.divide(0, 2.0) == 0 | ||||||||||||||||||||||||||||||||||||||||||||
| assert Calculator.divide(-4, 2.0) == -2.0 | ||||||||||||||||||||||||||||||||||||||||||||
| # assert Calculator.divide(2.0, 0.0) == 'Cannot divide by 0' | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| def test_add2(): | ||||||||||||||||||||||||||||||||||||||||||||
| # Test that add2 function raises NameError due to undefined variable 'z' | ||||||||||||||||||||||||||||||||||||||||||||
| with pytest.raises(NameError, match="name 'z' is not defined"): | ||||||||||||||||||||||||||||||||||||||||||||
| Calculator.add2(1, 2) | ||||||||||||||||||||||||||||||||||||||||||||
seer-by-sentry[bot] marked this conversation as resolved.
Show resolved
Hide resolved
Comment on lines
+33
to
+36
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The test is attempting to call Calculator.add2() which doesn't exist in the Calculator class. Based on the calculator.py file, there is no add2 method defined. This test will fail with an AttributeError before it can test for the expected NameError. Either implement the add2 method in the Calculator class or remove this test if it's not needed.
Suggested change
Did we get this right? 👍 / 👎 to inform future reviews. |
||||||||||||||||||||||||||||||||||||||||||||
| Calculator.add2(0, 0) | ||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+33
to
+37
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The test is attempting to call
Suggested change
Did we get this right? 👍 / 👎 to inform future reviews.
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmmm
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually in our team, we put all functions in a utils.py file
seer-by-sentry[bot] marked this conversation as resolved.
Show resolved
Hide resolved
Comment on lines
+33
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The test
Suggested change
Did we get this right? 👍 / 👎 to inform future reviews. |
||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test expects a
NameErrorwith the message "name 'z' is not defined", but this doesn't make sense in the context of testing a calculator method. This suggests the test is either:Tests should focus on the public API and expected behavior, not internal implementation errors.
Did we get this right? 👍 / 👎 to inform future reviews.