Skip to content

fix: Remove dead if sequence is None check and fix impossible test mocks#16

Open
Devguru-codes wants to merge 1 commit intoOSIPI:mainfrom
Devguru-codes:issue13
Open

fix: Remove dead if sequence is None check and fix impossible test mocks#16
Devguru-codes wants to merge 1 commit intoOSIPI:mainfrom
Devguru-codes:issue13

Conversation

@Devguru-codes
Copy link

@Devguru-codes Devguru-codes commented Feb 27, 2026

get_sequence() in factory.py never returns None — all code paths either return a valid sequence object or raise ValueError. This means:

  1. The if sequence is None guard in main.py:34 is dead code that can never execute.
  2. Two tests mock get_sequence to return None, creating impossible test scenarios that don't reflect actual behavior.

Fix

  1. Removed the dead if sequence is None check from main.py.
  2. Updated test mocks to use side_effect=ValueError(...) instead of return_value=None to test real failure behavior.

Files Changed

  • package/src/pyaslreport/main.py
  • package/src/pyaslreport/tests/test_package.py
# main.py
     sequence = get_sequence(modality, dicom_header)
     
-    if sequence is None:
-        raise ValueError(f"No matching sequence found for modality '{modality}' with the provided DICOM header")
-    
     return sequence.extract_bids_metadata()
# test_package.py
-         patch("pyaslreport.main.get_sequence", return_value=None):
+         patch("pyaslreport.main.get_sequence", side_effect=ValueError("No ASL sequence class found")):
         with pytest.raises(ValueError) as exc:
             get_bids_metadata(data)
-        assert "No matching sequence found" in str(exc.value)
+        assert "No ASL sequence class found" in str(exc.value)

Before/After Comparison

main branch (before fix)

# Test Status Detail
1 get_sequence can return None FAIL Never returns None
2 main.py has dead if sequence is None check FAIL Dead code present
3 Tests mock get_sequence with return_value=None FAIL Impossible mock

Summary: PASSED=0 FAILED=3

issue13 branch (after fix)

# Test Status Detail
1 get_sequence raises ValueError on failure (never returns None) PASS Confirmed correct behavior
2 main.py has dead if sequence is None check PASS Dead code removed
3 Tests mock get_sequence with return_value=None PASS Mocks now use side_effect=ValueError

Summary: PASSED=3 FAILED=0

Resolves #15

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.

Remove dead if sequence is None check and fix impossible test mocks in test_package.py

1 participant