-
Notifications
You must be signed in to change notification settings - Fork 28
improve coverage #541
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
improve coverage #541
Conversation
WalkthroughAdds new and expanded tests: a DirectoryIndexer.update paths test, a TDMS utilities test module for error handling and timestamp parsing, and a Terra15 unsupported version error test. No production code changes. Changes
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
tests/test_io/test_indexer.py(1 hunks)tests/test_io/test_tdms/test_tdms_utils.py(1 hunks)tests/test_io/test_terra15/test_terra15.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
tests/test_io/test_tdms/test_tdms_utils.py (1)
dascore/io/tdms/utils.py (2)
parse_time_stamp(87-99)type_not_supported(19-21)
tests/test_io/test_indexer.py (1)
dascore/io/indexer.py (2)
update(90-95)update(309-346)
tests/test_io/test_terra15/test_terra15.py (1)
dascore/io/terra15/utils.py (1)
_get_version_data_node(69-79)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: test_code_min_deps (macos-latest, 3.12)
- GitHub Check: test_code_min_deps (windows-latest, 3.12)
- GitHub Check: test_code_min_deps (windows-latest, 3.13)
- GitHub Check: test_code_min_deps (macos-latest, 3.13)
- GitHub Check: test_code_min_deps (ubuntu-latest, 3.12)
- GitHub Check: test_code_min_deps (ubuntu-latest, 3.13)
🔇 Additional comments (6)
tests/test_io/test_indexer.py (1)
279-297: LGTM! Good test for paths parameter coverage.This test effectively covers the
pathsparameter functionality of theDirectoryIndexer.update()method, testing both relative and absolute path scenarios. The test correctly verifies that the indexer can handle specific path filtering and returns appropriate results.tests/test_io/test_terra15/test_terra15.py (2)
6-6: Appropriate import addition.The
ClassVarimport is correctly added to support the mock object's type annotation in the new test method.
75-87: Excellent test for error handling coverage.This test effectively covers the error path in
_get_version_data_nodefunction for unsupported Terra15 versions. The mock approach is clean and appropriate for testing this specific error condition.tests/test_io/test_tdms/test_tdms_utils.py (3)
13-18: Good coverage for error function.This test correctly verifies that
type_not_supportedraises the expectedNotImplementedErrorwith the proper message, providing complete coverage for this utility function.
20-34: Comprehensive testing of edge cases.This test method thoroughly covers the various invalid input scenarios for
parse_time_stamp, including zero values andNoneinputs. The test cases align well with the function's logic that returnsNonewhenfractions + seconds <= 0or when either parameter isNone.
36-48: Effective test for valid timestamp parsing.This test verifies the positive path of
parse_time_stampwith a realistic timestamp calculation (100 years after the 1904 LabVIEW epoch). The assertions appropriately check both the return type and that the resulting year is valid.
| # Use a timestamp that represents a valid date after 1904 | ||
| seconds = 365 * 24 * 3600 * 100 # 100 years after 1904 | ||
| fractions = 0.5 # Some fractional seconds |
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.
🧩 Analysis chain
Verify the timestamp calculation accuracy.
The test uses 100 years after 1904 which should result in a 2004 date. LabVIEW's timestamp epoch starts at January 1, 1904 00:00:00.00 UTC, so the calculation 365 * 24 * 3600 * 100 (100 years of seconds) is reasonable for testing purposes.
Let me verify the exact calculation is correct:
🏁 Script executed:
#!/bin/bash
# Description: Verify the timestamp calculation for 100 years after 1904
# Calculate seconds in 100 years (accounting for leap years)
python3 << 'EOF'
import datetime
# Calculate the exact number of seconds between 1904-01-01 and 2004-01-01
start_date = datetime.datetime(1904, 1, 1)
end_date = datetime.datetime(2004, 1, 1)
diff = end_date - start_date
seconds_in_100_years = int(diff.total_seconds())
print(f"Exact seconds in 100 years (1904-2004): {seconds_in_100_years}")
print(f"Test value used: {365 * 24 * 3600 * 100}")
print(f"Difference: {seconds_in_100_years - (365 * 24 * 3600 * 100)}")
# Test the calculation with the exact value
test_seconds = 365 * 24 * 3600 * 100 # 100 years after 1904
test_fractions = 0.5
result_date = datetime.timedelta(0, test_fractions * 2**-64 + test_seconds) + datetime.datetime(1904, 1, 1)
print(f"Resulting date from test: {result_date}")
print(f"Year: {result_date.year}")
EOFLength of output: 155
Fix timestamp calculation: account for leap years (use exact seconds or datetime arithmetic).
365 * 24 * 3600 * 100 = 3,153,600,000 but the exact seconds 1904‑01‑01 → 2004‑01‑01 = 3,155,760,000 (diff 2,160,000s = 25 days). With the current value the test produces 2003‑12‑07 instead of 2004‑01‑01. Replace the constant with 3,155,760,000 or compute the timestamp from datetime(1904,1,1) + timedelta(...) to avoid the off-by-leap-year error.
tests/test_io/test_tdms/test_tdms_utils.py:42-44
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #541 +/- ##
==========================================
+ Coverage 99.88% 99.92% +0.03%
==========================================
Files 124 124
Lines 10336 10336
==========================================
+ Hits 10324 10328 +4
+ Misses 12 8 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Description
This PR improves DASCore's test coverage slightly.
Checklist
I have (if applicable):
Summary by CodeRabbit