Skip to content

Deterministically cover the scan-time DependencyError branch - #852

Merged
d-chambers merged 1 commit into
devfrom
cover-scan-dependency-error
Aug 10, 2026
Merged

Deterministically cover the scan-time DependencyError branch#852
d-chambers merged 1 commit into
devfrom
cover-scan-dependency-error

Conversation

@d-chambers

@d-chambers d-chambers commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Description

dev's codecov/project check is failing at 99.98% (target 100%), which hard-fails every open PR (it already showed up on #846 and #851). The two uncovered lines are the except DependencyError warn-and-continue branch in the scan loop (dascore/io/core.py:1393-1394).

That branch was only exercised indirectly by test_legacy_file_scan_warns_and_skips, which scans the legacy DASVader JLD2 file — and whether that raises DependencyError depends on the installed HDF5 stack. An upstream dependency update (first visible on the #845 merge CI run) made the legacy file scan cleanly, so the branch silently lost coverage.

This adds a deterministic test using a name-gated _DependencyErrorFormatter, mirroring the existing _MissingOptionalFormatter pattern, so the branch stays covered regardless of the HDF5 stack. Test-only change.

Changelog

none

Checklist

I have (if applicable):

  • referenced the GitHub issue this PR closes.
  • documented the new feature with docstrings and/or appropriate doc page.
  • included tests. See testing guidelines.
  • added the "ready_for_review" tag once the PR is ready to be reviewed.

Summary by CodeRabbit

  • Bug Fixes

    • Improved handling of dependency-related file errors during scanning.
    • Dependency errors now generate warnings without preventing other readable files from being processed.
  • Tests

    • Added coverage to verify warning behavior and continued scanning when dependency issues are encountered.

The warn-and-continue branch for DependencyError in the scan loop was
only exercised by the legacy DASVader file, and only on HDF5 stacks
that cannot dereference it; a stack update flipped that and left dev
at 99.98% project coverage, hard-failing codecov for every open PR.
Cover it with a name-gated formatter like the missing-optional one.
@d-chambers d-chambers added the ready_for_review PR is ready for review label Aug 10, 2026
@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5648147c-92e5-4177-b4d9-1a0ee6875908

📥 Commits

Reviewing files that changed from the base of the PR and between a005012 and c7f53c4.

📒 Files selected for processing (1)
  • tests/test_io/test_io_core.py

📝 Walkthrough

Walkthrough

Changes

The test suite now simulates DependencyError during scanning. It verifies that the error emits a warning, skips the affected file, and allows other readable files to produce results.

Dependency error scan handling

Layer / File(s) Summary
Dependency error simulation and scan assertions
tests/test_io/test_io_core.py
Adds the DependencyError import, a test formatter that raises the error for .dep files, and coverage for warning, skip, and fallback scan behavior.

Suggested labels: CI

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly explains the problem, solution, scope, and test coverage, and it completes the applicable checklist items.
Title check ✅ Passed The title clearly and concisely identifies the deterministic test coverage added for the scan-time DependencyError branch.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch cover-scan-dependency-error

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot added the CI continuous integration label Aug 10, 2026
@d-chambers
d-chambers merged commit 94d149b into dev Aug 10, 2026
28 checks passed
@d-chambers
d-chambers deleted the cover-scan-dependency-error branch August 10, 2026 10:40
@codecov

codecov Bot commented Aug 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (a005012) to head (c7f53c4).
⚠️ Report is 1 commits behind head on dev.

Additional details and impacted files
@@             Coverage Diff             @@
##              dev      #852      +/-   ##
===========================================
+ Coverage   99.98%   100.00%   +0.01%     
===========================================
  Files         164       164              
  Lines       18192     18192              
===========================================
+ Hits        18190     18192       +2     
+ Misses          2         0       -2     
Flag Coverage Δ
network 48.30% <ø> (ø)
unittests 100.00% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@d-chambers d-chambers removed the ready_for_review PR is ready for review label Aug 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI continuous integration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant