Skip to content

tests: remove duplicate and dead test cases#8896

Open
aymuos15 wants to merge 2 commits into
Project-MONAI:devfrom
aymuos15:cleanup/redundant-tests
Open

tests: remove duplicate and dead test cases#8896
aymuos15 wants to merge 2 commits into
Project-MONAI:devfrom
aymuos15:cleanup/redundant-tests

Conversation

@aymuos15
Copy link
Copy Markdown
Contributor

@aymuos15 aymuos15 commented Jun 3, 2026

Description

Test-only cleanup. Removes byte-identical duplicate cases and one permanently-skipped test. No production code or test logic changes.

Changes

Duplicate within-file parametrized cases (identical entries that asserted the same thing twice, so removal changes nothing covered):

  • tests/losses/test_unified_focal_loss.py: both TEST_CASES entries were identical
  • tests/inferers/test_sliding_window_inference.py: duplicate 3D small roi case
  • tests/losses/test_dice_loss.py: sigmoid case repeated
  • tests/losses/test_generalized_dice_loss.py: sigmoid case repeated
  • tests/transforms/utility/test_apply_transform_to_pointsd.py: duplicate entry

Dead test removed:

  • tests/apps/test_download_url_yandex.py: removed test_verify and its now-unused YANDEX_MODEL_URL. It was permanently @unittest.skip-ed ("data source unstable") and hits an external Yandex URL, so it never runs in CI. The error path stays covered by test_verify_error.

Types of changes

  • Non-breaking change (test-only cleanup)
  • In-line documentation / comments updated as needed
  • All tests passing locally

aymuos15 added 2 commits June 3, 2026 16:18
Several parametrized test lists contained byte-identical entries that
asserted the same thing twice, contributing no extra coverage:

- losses/test_unified_focal_loss.py: both TEST_CASES entries identical
- inferers/test_sliding_window_inference.py: duplicate 3D small-roi case
- losses/test_dice_loss.py: sigmoid case repeated
- losses/test_generalized_dice_loss.py: sigmoid case repeated
- transforms/utility/test_apply_transform_to_pointsd.py: duplicate entry

Signed-off-by: Soumya Snigdha Kundu <soumya_snigdha.kundu@kcl.ac.uk>
apps/test_download_url_yandex.py: remove test_verify, which depends on an
unstable external Yandex URL and is permanently @unittest.skip-ed, so it
never runs in CI. The error path remains covered by test_verify_error.

Note: test_smartcachedataset::test_set_data was left skipped on purpose.
Issue Project-MONAI#5660 (the smartcache thread hanging for hours on CI) was closed as
stale with no fix, so the intermittent hang risk likely remains.

Signed-off-by: Soumya Snigdha Kundu <soumya_snigdha.kundu@kcl.ac.uk>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jun 3, 2026

📝 Walkthrough

Walkthrough

This PR removes test coverage across five test modules. The changes eliminate one unreliable Yandex download test and its constant; prune one 3D sliding-window inference scenario; remove single parameterized cases from GeneralizedDiceLoss, UnifiedFocalLoss, and point-transform coordinate tests. All changes are deletions from test data or test methods with no alterations to core logic.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed Title accurately and concisely summarizes the main change: removing duplicate and dead test cases.
Description check ✅ Passed Description covers the purpose, specific changes, and types of changes checked; aligned well with template requirements.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with 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.

❤️ Share

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

Copy link
Copy Markdown
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.

🧹 Nitpick comments (1)
tests/losses/test_unified_focal_loss.py (1)

22-30: ⚡ Quick win

Test coverage now minimal after duplicate removal.

Only one test case remains, covering perfect prediction (loss = 0.0). Loss functions need diverse scenarios: imperfect predictions, non-zero loss values, parameter variations (weight, gamma, delta), edge cases.

Suggested additional test cases
TEST_CASES = [
    [  # shape: (2, 1, 2, 2), (2, 1, 2, 2) - perfect prediction
        {
            "y_pred": torch.tensor([[[[1.0, 0], [0, 1.0]]], [[[1.0, 0], [0, 1.0]]]]),
            "y_true": torch.tensor([[[[1.0, 0], [0, 1.0]]], [[[1.0, 0], [0, 1.0]]]]),
        },
        0.0,
    ],
    [  # imperfect prediction - partial mismatch
        {
            "y_pred": torch.tensor([[[[0.8, 0.2], [0.3, 0.9]]], [[[0.7, 0.1], [0.2, 0.85]]]]),
            "y_true": torch.tensor([[[[1.0, 0], [0, 1.0]]], [[[1.0, 0], [0, 1.0]]]]),
        },
        # expected_val would need to be calculated based on loss formula
    ],
    # Add cases for different weight, gamma, delta parameters
]
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/losses/test_unified_focal_loss.py` around lines 22 - 30, Replace the
single perfect-prediction entry in TEST_CASES with multiple diverse cases: keep
the existing perfect case, add at least one imperfect-prediction case (modify
y_pred vs y_true to produce non-zero loss), add edge cases (all-zeros/constant
predictions, class-imbalance patterns), and add cases that exercise different
loss parameters (pass explicit weight, gamma, delta variations to the
UnifiedFocalLoss call). Ensure each new case in TEST_CASES references the same
keys ("y_pred", "y_true") and provides either a precomputed expected value (for
deterministic settings) or mark it for approximate/asserted range checks when
exact numeric values are complex; use the symbols TEST_CASES, y_pred, y_true and
the loss parameters weight/gamma/delta to locate where to modify tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@tests/losses/test_unified_focal_loss.py`:
- Around line 22-30: Replace the single perfect-prediction entry in TEST_CASES
with multiple diverse cases: keep the existing perfect case, add at least one
imperfect-prediction case (modify y_pred vs y_true to produce non-zero loss),
add edge cases (all-zeros/constant predictions, class-imbalance patterns), and
add cases that exercise different loss parameters (pass explicit weight, gamma,
delta variations to the UnifiedFocalLoss call). Ensure each new case in
TEST_CASES references the same keys ("y_pred", "y_true") and provides either a
precomputed expected value (for deterministic settings) or mark it for
approximate/asserted range checks when exact numeric values are complex; use the
symbols TEST_CASES, y_pred, y_true and the loss parameters weight/gamma/delta to
locate where to modify tests.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: caf5c6f5-2344-48f2-8ab8-c2030b22206b

📥 Commits

Reviewing files that changed from the base of the PR and between 013bda0 and 9fefa78.

📒 Files selected for processing (6)
  • tests/apps/test_download_url_yandex.py
  • tests/inferers/test_sliding_window_inference.py
  • tests/losses/test_dice_loss.py
  • tests/losses/test_generalized_dice_loss.py
  • tests/losses/test_unified_focal_loss.py
  • tests/transforms/utility/test_apply_transform_to_pointsd.py
💤 Files with no reviewable changes (5)
  • tests/inferers/test_sliding_window_inference.py
  • tests/transforms/utility/test_apply_transform_to_pointsd.py
  • tests/apps/test_download_url_yandex.py
  • tests/losses/test_generalized_dice_loss.py
  • tests/losses/test_dice_loss.py

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.

1 participant