Skip to content
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

Increase test coverage #78

Merged
merged 9 commits into from
May 8, 2024
Merged

Increase test coverage #78

merged 9 commits into from
May 8, 2024

Conversation

emdavis02
Copy link
Collaborator

@emdavis02 emdavis02 commented May 3, 2024

  • added tests for case when Series class cannot find a file needed for Series.load()
  • added error code for when video is not found in Series.load()

Summary by CodeRabbit

  • New Features
    • Added a check to verify file existence before loading a video file, enhancing reliability during video operations.
  • Tests
    • Introduced new test cases to handle various prediction scenarios and video loading errors, improving robustness and error handling.

Copy link
Contributor

coderabbitai bot commented May 3, 2024

Walkthrough

The latest update enhances the robustness of the video loading process in sleap_roots/series.py by introducing a file existence check prior to loading. In tandem, tests/test_series.py has been enriched with new imports and test functions aimed at validating prediction scenarios and handling video loading errors more effectively.

Changes

Files Change Summary
sleap_roots/series.py Added file existence check before video loading.
tests/test_series.py New imports (redirect_stdout, io) and test functions for various prediction and error scenarios.

🐇✨
In the meadow of code, under the moon's glow,
A rabbit hops, leaving traces in the snow.
Each paw print a test, each leap a new check,
Ensuring no video loads amiss, oh what the heck!
Celebrate the changes, for they make our code spry,
Hopping towards perfection, under the vast sky.
🌌📼


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

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 as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Out of diff range and nitpick comments (2)
tests/test_series.py (2)

Line range hint 2-2: Remove the unused import of numpy.

- import numpy as np

This import is unused and should be removed to clean up the code.


Line range hint 142-142: Rename the redefined test_series_name function to avoid confusion.

- def test_series_name(dummy_series):
+ def test_series_name_from_dummy(dummy_series):

This change avoids the redefinition of test_series_name, which is already defined earlier in the file.

Comment on lines 61 to 76
def test_primary_prediction_not_found(tmp_path):
dummy_video_path = tmp_path / "dummy_video.mp4"
dummy_video_path.write_text("This is a dummy video file.")

# Create a dummy Series instance with a non-existent primary prediction file
output = io.StringIO()
with redirect_stdout(output):
Series.load(h5_path=str(dummy_video_path), primary_name="nonexistent")

# format file path string for assert statement
path_obj = Path(dummy_video_path)
parent_dir = path_obj.parent
filename_without_extension = path_obj.stem
new_file_path = parent_dir / filename_without_extension

assert output.getvalue() == f"Primary prediction file not found: {new_file_path}.nonexistent.predictions.slp\n"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simplify the path manipulation in the error message construction.

-    path_obj = Path(dummy_video_path)
-    parent_dir = path_obj.parent
-    filename_without_extension = path_obj.stem
-    new_file_path = parent_dir / filename_without_extension
+    new_file_path = Path(dummy_video_path).with_suffix('')

This change uses the with_suffix('') method to remove the extension from the path, simplifying the code and achieving the same result.


Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
def test_primary_prediction_not_found(tmp_path):
dummy_video_path = tmp_path / "dummy_video.mp4"
dummy_video_path.write_text("This is a dummy video file.")
# Create a dummy Series instance with a non-existent primary prediction file
output = io.StringIO()
with redirect_stdout(output):
Series.load(h5_path=str(dummy_video_path), primary_name="nonexistent")
# format file path string for assert statement
path_obj = Path(dummy_video_path)
parent_dir = path_obj.parent
filename_without_extension = path_obj.stem
new_file_path = parent_dir / filename_without_extension
assert output.getvalue() == f"Primary prediction file not found: {new_file_path}.nonexistent.predictions.slp\n"
def test_primary_prediction_not_found(tmp_path):
dummy_video_path = tmp_path / "dummy_video.mp4"
dummy_video_path.write_text("This is a dummy video file.")
# Create a dummy Series instance with a non-existent primary prediction file
output = io.StringIO()
with redirect_stdout(output):
Series.load(h5_path=str(dummy_video_path), primary_name="nonexistent")
# format file path string for assert statement
new_file_path = Path(dummy_video_path).with_suffix('')
assert output.getvalue() == f"Primary prediction file not found: {new_file_path}.nonexistent.predictions.slp\n"

tests/test_series.py Outdated Show resolved Hide resolved
tests/test_series.py Outdated Show resolved Hide resolved
Copy link

codecov bot commented May 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.18%. Comparing base (9516f47) to head (ceb9570).

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #78      +/-   ##
==========================================
+ Coverage   74.62%   75.18%   +0.55%     
==========================================
  Files          13       13              
  Lines        1344     1346       +2     
==========================================
+ Hits         1003     1012       +9     
+ Misses        341      334       -7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Out of diff range and nitpick comments (2)
tests/test_series.py (2)

Line range hint 2-2: Remove unused import to clean up the code.

- import numpy as np

Line range hint 159-159: Rename the function to avoid redefinition and potential test execution issues.

- def test_series_name(series_instance):
+ def test_series_name_from_instance(series_instance):

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Out of diff range and nitpick comments (2)
tests/test_series.py (2)

Line range hint 2-2: Remove unused import to clean up the code.

- import numpy as np

Line range hint 150-150: Rename the function to avoid redefinition and potential test suite issues.

- def test_series_name(series_instance):
+ def test_series_name_from_instance(series_instance):
Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between 88fbc3f and 887cbe4.
Files selected for processing (1)
  • tests/test_series.py (3 hunks)
Additional Context Used
Ruff (2)
tests/test_series.py (2)

2-2: numpy imported but unused


150-150: Redefinition of unused test_series_name from line 131

Additional comments not posted (5)
tests/test_series.py (5)

62-77: LGTM! Good use of fixtures and stdout redirection for testing error handling.


80-95: LGTM! Consistent approach in testing different prediction types.


98-113: LGTM! Consistent and clear testing methodology across different prediction types.


116-128: LGTM! Properly tests the error handling for video loading.


159-167: LGTM! Effective testing of error handling for CSV path issues.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Out of diff range and nitpick comments (5)
tests/test_series.py (5)

Line range hint 2-2: Remove unused import to clean up the code.

- import numpy as np

62-77: Ensure consistent error message formatting across similar test functions.

Consider using a helper function to format the error message to ensure consistency and reduce duplication.


80-95: Ensure consistent error message formatting across similar test functions.

Consider using a helper function to format the error message to ensure consistency and reduce duplication.


98-113: Ensure consistent error message formatting across similar test functions.

Consider using a helper function to format the error message to ensure consistency and reduce duplication.


Line range hint 150-150: Remove the redefinition of the function test_series_name to avoid confusion and potential errors.

- def test_series_name(series_instance):
-     assert series_instance.series_name == "dummy"
Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between 887cbe4 and 297f4ba.
Files selected for processing (1)
  • tests/test_series.py (3 hunks)
Additional Context Used
Ruff (2)
tests/test_series.py (2)

2-2: numpy imported but unused


150-150: Redefinition of unused test_series_name from line 131

Additional comments not posted (2)
tests/test_series.py (2)

7-8: Import statements for redirect_stdout and io are correctly added to support the new test functionalities.


116-128: Correctly handles the scenario where an invalid video file path is provided.

Copy link
Collaborator

@eberrigan eberrigan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! Just always use Path from pathlib to make file paths accessible across platforms. It should pass the tests after that.

tests/test_series.py Outdated Show resolved Hide resolved
tests/test_series.py Outdated Show resolved Hide resolved
tests/test_series.py Outdated Show resolved Hide resolved
tests/test_series.py Outdated Show resolved Hide resolved
tests/test_series.py Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Out of diff range and nitpick comments (2)
tests/test_series.py (2)

Line range hint 2-2: Remove unused import to clean up the code.

- import numpy as np

Line range hint 150-150: Avoid redefining the function test_series_name which was previously defined at line 131. Consider renaming one of the functions or consolidating their logic if applicable.

- def test_series_name(series_instance):
+ def test_series_name_from_instance(series_instance):
Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between 297f4ba and bdcf391.
Files selected for processing (1)
  • tests/test_series.py (3 hunks)
Additional Context Used
Ruff (2)
tests/test_series.py (2)

2-2: numpy imported but unused


150-150: Redefinition of unused test_series_name from line 131

Additional comments not posted (6)
tests/test_series.py (6)

7-8: The import statements for redirect_stdout and io are correctly added to support the new test functionalities.


62-77: The test function test_primary_prediction_not_found correctly simulates the scenario where the primary prediction file is not found. The use of redirect_stdout to capture the output for assertion is appropriate.


80-95: The test function test_lateral_prediction_not_found correctly simulates the scenario where the lateral prediction file is not found. The structure and logic are consistent with other similar test functions.


98-113: The test function test_crown_prediction_not_found correctly simulates the scenario where the crown prediction file is not found. This maintains consistency in testing various label types.


116-128: The test function test_video_loading_error effectively checks for errors during video file loading, ensuring that the error handling in the Series.load method is robust.


159-167: The test function test_expected_count_error correctly simulates the scenario where the CSV path is invalid. The use of redirect_stdout to capture and assert the error message is consistent with other tests.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Out of diff range and nitpick comments (2)
tests/test_series.py (2)

Line range hint 2-2: Remove unused import to clean up the code.

- import numpy as np

Line range hint 150-150: Remove or rename this function to avoid redefinition and potential test suite issues.

Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between bdcf391 and 6eb8a07.
Files selected for processing (1)
  • tests/test_series.py (3 hunks)
Additional Context Used
Ruff (2)
tests/test_series.py (2)

2-2: numpy imported but unused


150-150: Redefinition of unused test_series_name from line 131

Additional comments not posted (10)
tests/test_series.py (10)

Line range hint 10-13: This fixture is correctly implemented.


Line range hint 17-20: This fixture is correctly implemented.


Line range hint 28-32: This fixture is correctly implemented.


Line range hint 36-44: This fixture is correctly implemented.


Line range hint 48-54: This fixture is correctly implemented.


62-77: This test function is correctly implemented and checks the error handling effectively.


80-95: This test function is correctly implemented and checks the error handling effectively.


98-113: This test function is correctly implemented and checks the error handling effectively.


116-128: This test function is correctly implemented and checks the error handling effectively.


159-167: This test function is correctly implemented and checks the error handling effectively.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Out of diff range and nitpick comments (2)
tests/test_series.py (2)

Line range hint 2-2: Remove unused import to clean up the code.

- import numpy as np

Line range hint 151-151: Redefinition of function test_series_name could lead to confusion and test failures.

Consider renaming one of the functions to avoid conflicts.

Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between 6eb8a07 and ceb9570.
Files selected for processing (1)
  • tests/test_series.py (3 hunks)
Additional Context Used
Ruff (2)
tests/test_series.py (2)

2-2: numpy imported but unused


151-151: Redefinition of unused test_series_name from line 132

Additional comments not posted (5)
tests/test_series.py (5)

62-78: The test function for handling non-existent primary prediction files is well-implemented.


81-96: The test function for handling non-existent lateral prediction files is correctly implemented.


99-114: The test function for handling non-existent crown prediction files is correctly implemented.


117-129: The test function for handling video loading errors is correctly implemented.


160-168: The test function for handling errors related to an invalid CSV path is correctly implemented.

@eberrigan eberrigan merged commit 151ddb3 into main May 8, 2024
5 checks passed
@eberrigan eberrigan deleted the elise/improve-tests branch May 8, 2024 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants