test#6
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements several data parsing and algorithmic functions for a flight ticket system and reactor monitoring system, along with corresponding unit tests. The changes replace placeholder pass statements with working implementations.
Key Changes:
- Implements 5 core functions: departure airport extraction, baggage allowance checking, flight number validation, leap year calculation, and reactor status monitoring
- Adds comprehensive unit tests for the reactor_status function in a new test file
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| system_check.py | Implements get_departure_airport, check_baggage_allowance, validate_flight_number, is_leap_year, and reactor_status functions |
| test_reactor.py | Adds unit tests for reactor_status function covering normal, invalid, critical, unstable, and maintenance scenarios |
Comments suppressed due to low confidence (1)
system_check.py:134
- Test is always false, because of this condition.
elif temp < 0 or radiation < 0:
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
|
|
||
|
|
||
| class TestReactor (unittest.TestCase): |
There was a problem hiding this comment.
Extra space before parenthesis in class name. Should be TestReactor(unittest.TestCase): without the space.
| class TestReactor (unittest.TestCase): | |
| class TestReactor(unittest.TestCase): |
| if temp < 500: | ||
| return "Maintenance Mode" | ||
| elif temp < 0 or radiation < 0: | ||
| return "Sensor Error" |
There was a problem hiding this comment.
Logic order error: checking temp < 500 before temp < 0 means negative temperatures will incorrectly return 'Maintenance Mode' instead of 'Sensor Error'. The sensor error check (line 134-135) must come first to properly handle negative values.
| if temp < 500: | |
| return "Maintenance Mode" | |
| elif temp < 0 or radiation < 0: | |
| return "Sensor Error" | |
| if temp < 0 or radiation < 0: | |
| return "Sensor Error" | |
| elif temp < 500: | |
| return "Maintenance Mode" |
| return "Sensor Error" | ||
| elif temp > 2000 or radiation > 500: | ||
| return "CRITICAL" | ||
| elif 1000 < temp < 2000 and radiation > 100: |
There was a problem hiding this comment.
Incorrect range check excludes boundary values. According to the docstring specifying 'between 1000 and 2000 (inclusive)', this should be 1000 <= temp <= 2000 to include both boundaries.
| elif 1000 < temp < 2000 and radiation > 100: | |
| elif 1000 <= temp <= 2000 and radiation > 100: |
| elif 1000 < temp < 2000 and radiation > 100: | ||
| return "WARNING" | ||
| else: | ||
| return "Normal Mode" |
There was a problem hiding this comment.
Incorrect return value. According to the docstring and test expectations (line 59 in test_system_check.py expects 'Normal Operation'), this should return 'Normal Operation' not 'Normal Mode'.
| return "Normal Mode" | |
| return "Normal Operation" |
| if year % 100 == 0: | ||
| if year % 4 == 0: | ||
| return True | ||
| return False |
There was a problem hiding this comment.
Leap year logic is inverted. The correct implementation should check divisibility by 4 first, then 100, then 400. A year divisible by 400 is always a leap year (e.g., 2000), but the current nested logic will fail for years like 2024 (divisible by 4 but not 400). The function should be: if divisible by 400, return True; elif divisible by 100, return False; elif divisible by 4, return True; else return False.
| if year % 100 == 0: | |
| if year % 4 == 0: | |
| return True | |
| return False | |
| return True | |
| elif year % 100 == 0: | |
| return False | |
| elif year % 4 == 0: | |
| return True | |
| else: | |
| return False |
| checker = int(flight_info[-1]) | ||
|
|
||
| if checker % 2 == 0: | ||
| return "Valid - Northbound" | ||
| if checker % 2 != 0: | ||
| return "Valid - Southbound" | ||
| else: | ||
| return "Invalid flight" #probably had to use exceptions somewhere here |
There was a problem hiding this comment.
Unreachable else clause at line 70-71. Since line 68 checks checker % 2 != 0 (which covers all cases not covered by line 66), the else block will never execute. Additionally, line 71 returns 'Invalid flight' but should return 'Invalid Flight' (capital F) per the docstring. The function also doesn't handle the ValueError exception when int(flight_info[-1]) fails for non-numeric characters.
| checker = int(flight_info[-1]) | |
| if checker % 2 == 0: | |
| return "Valid - Northbound" | |
| if checker % 2 != 0: | |
| return "Valid - Southbound" | |
| else: | |
| return "Invalid flight" #probably had to use exceptions somewhere here | |
| try: | |
| checker = int(flight_info[-1]) | |
| except ValueError: | |
| return "Invalid Flight" | |
| if checker % 2 == 0: | |
| return "Valid - Northbound" | |
| else: | |
| return "Valid - Southbound" |
| if checker % 2 != 0: | ||
| return "Valid - Southbound" | ||
| else: | ||
| return "Invalid flight" #probably had to use exceptions somewhere here |
There was a problem hiding this comment.
Return value has incorrect capitalization. Should be 'Invalid Flight' (capital F) to match the specification in the function docstring at line 59.
| return "Invalid flight" #probably had to use exceptions somewhere here | |
| return "Invalid Flight" #probably had to use exceptions somewhere here |
| checker = int(flight_info[-1]) | ||
|
|
||
| if checker % 2 == 0: | ||
| return "Valid - Northbound" | ||
| if checker % 2 != 0: | ||
| return "Valid - Southbound" | ||
| else: | ||
| return "Invalid flight" #probably had to use exceptions somewhere here |
There was a problem hiding this comment.
Missing exception handling for invalid input. Line 64 will raise a ValueError if the last character of flight_info is not numeric (e.g., 'JOABC'), but this exception is not caught. According to the test at line 41 in test_system_check.py, 'FL-JOABC-JNB-CPT' should return 'Invalid Flight'. Wrap the int conversion in a try-except block to catch ValueError and return 'Invalid Flight'.
| checker = int(flight_info[-1]) | |
| if checker % 2 == 0: | |
| return "Valid - Northbound" | |
| if checker % 2 != 0: | |
| return "Valid - Southbound" | |
| else: | |
| return "Invalid flight" #probably had to use exceptions somewhere here | |
| try: | |
| checker = int(flight_info[-1]) | |
| except ValueError: | |
| return "Invalid Flight" | |
| if checker % 2 == 0: | |
| return "Valid - Northbound" | |
| if checker % 2 != 0: | |
| return "Valid - Southbound" | |
| else: | |
| return "Invalid Flight" |
Ayanda