-
Notifications
You must be signed in to change notification settings - Fork 34
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
more tests! #138
more tests! #138
Conversation
Reviewer's Guide by SourceryThis pull request adds more tests to the project, focusing on improving test coverage for the art functionality, simple GPT integration, and some refactoring in existing tests. The changes include new test files, modifications to existing test files, and updates to import statements. File-Level Changes
Tips
|
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.
Hey @ryansurf - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider enhancing the test cases, especially for the GPT and art functionalities, to be more comprehensive. Current tests only check for basic output presence rather than specific content or behavior.
- There's an inconsistency in import styles (e.g., changing from
from src.helper import extract_decimal
tofrom src import helper
). Could you explain the reasoning behind this change and consider applying it consistently across the codebase if it's intentional?
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟡 Testing: 1 issue found
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
tests/test_art.py
Outdated
def test_print_wave(): | ||
""" | ||
Testing the print_wave() function | ||
Uses sys & io to capture printed output | ||
""" | ||
# Capture the output | ||
captured_output = io.StringIO() # Create StringIO object | ||
sys.stdout = captured_output # Redirect stdout. | ||
|
||
# Call the function | ||
art.print_wave(1, 0, "blue") | ||
|
||
# Reset redirect. | ||
sys.stdout = sys.__stdout__ | ||
|
||
# Now captured_output.getvalue() contains the printed content | ||
output = captured_output.getvalue() | ||
|
||
# Perform assertions based on expected output | ||
assert output != "", "print_wave() did not print anything" |
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.
suggestion (testing): Improve test_print_wave() with more specific assertions
The current test only checks if something was printed. Consider adding assertions to verify the content of the output, such as checking for specific wave characters or color codes.
def test_print_wave(): | |
""" | |
Testing the print_wave() function | |
Uses sys & io to capture printed output | |
""" | |
# Capture the output | |
captured_output = io.StringIO() # Create StringIO object | |
sys.stdout = captured_output # Redirect stdout. | |
# Call the function | |
art.print_wave(1, 0, "blue") | |
# Reset redirect. | |
sys.stdout = sys.__stdout__ | |
# Now captured_output.getvalue() contains the printed content | |
output = captured_output.getvalue() | |
# Perform assertions based on expected output | |
assert output != "", "print_wave() did not print anything" | |
def test_print_wave(): | |
captured_output = io.StringIO() | |
sys.stdout = captured_output | |
art.print_wave(1, 0, "blue") | |
sys.stdout = sys.__stdout__ | |
output = captured_output.getvalue() | |
assert "~" in output, "Wave character not found in output" | |
assert "\033[94m" in output, "Blue color code not found in output" | |
assert output.count("\n") == 1, "Incorrect number of lines printed" |
Codecov ReportAll modified and coverable lines are covered by tests ✅ |
General:
Code:
More tests have been added! The art is now tested, along with the simple gpt.
Summary by Sourcery
Add new tests for the art and gpt modules, refactor import statements in test_helper.py, and enhance test coverage for the extract_decimal function.
New Features:
Enhancements:
Tests: