Replace slides on unittest by slides on pytest #16
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ering into development
…ering into development
Have been replaced by slides on pytest
Reviewer's Guide by SourceryThis pull request replaces the unit test implementation with pytest. It also introduces custom markers and parameterization to the tests. Sequence diagram for parameterized test executionsequenceDiagram
participant P as Pytest Runner
participant T as Test Function
participant F as Fibonacci Function
loop For each test parameter
P->>T: Run test with parameter
T->>F: Call fib(n)
F-->>T: Return result
T-->>P: Assert result matches expected
end
Class diagram for Fibonacci implementation with test structureclassDiagram
class fibonacci {
+fib(n: int): int
}
class test_fib {
+test_negative_input()
+test_base_cases()
+test_recursive_case()
}
note for test_fib "Tests can be marked with @pytest.mark.slow"
test_fib ..> fibonacci : tests
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @gjbex - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 3 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| def fib(n): | ||
| if n < 0: | ||
| raise ValueError('fac argument must be positive') |
There was a problem hiding this comment.
issue (typo): Error message refers to 'fac argument' but this is a Fibonacci function
This appears to be a copy-paste error. Consider changing to 'fibonacci argument' or simply 'n' for clarity.
Suggested change
| raise ValueError('fac argument must be positive') | |
| raise ValueError('fibonacci argument must be positive') |
| if __name__ == '__main__': | ||
| from argparse import ArgumentParser | ||
| arg_parser = ArgumentParser(description='compute fibonacci numbers') | ||
| arg_parser.add_argument('n', type=int, default=5, |
There was a problem hiding this comment.
issue (bug_risk): Default value won't work for positional argument
The default parameter has no effect on positional arguments. Consider using an optional argument (--n) if a default value is needed.
| # Typing | ||
|
|
||
| Python 3.5 introducd optional type annotation for functions, and that | ||
| Python 3.5 introduced optional type annotation for functions, and that |
There was a problem hiding this comment.
issue (typo): Typo: "introducd" should be "introduced".
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Introduce examples for using custom markers and parameterized tests with pytest.
Tests: