1163 support signal[t] | none syntax in devicefiller#1165
Conversation
| async def connect_mock(self, device: Device, mock: LazyMock): | ||
| self.filler.create_device_vector_entries_to_mock(self.mock_device_vector_len) | ||
| # Set the name of the device to name all children | ||
| self.filler.check_filled("") |
There was a problem hiding this comment.
Why is this needed? All optional devices should have been created in mock mode so there should be no need to call this function...
There was a problem hiding this comment.
I added this so that when trying to connect check_filled would be called testing the added code to create the disconnected attr. Otherwise no other part of the mock code calls for filler.check_filled().
There was a problem hiding this comment.
Ok, please remove this line and add a test that instantiates DeviceFiller manually on a Device with both mandatory and optional Signals, then calls check_filled to check its failure and success paths.
| await device.connect(mock=True) | ||
|
|
||
| # After connecting if the signal is not present it's set to None | ||
| assert device.signal_rw is None |
There was a problem hiding this comment.
I would expect the signal to exist here
There was a problem hiding this comment.
You're right, that was my mistake, I'll fix it.
|
@claude suggest code for the test mentioned in #1165 (comment) |
|
Claude finished @coretl's task in 2m 40s —— View job Test implementation completedTasks:
Changes MadeI've implemented the test as requested in the review comment: 1. Removed unnecessary line from
2. Added comprehensive test in
3. Updated comment in
The test verifies that
--- | Branch |
- Remove unnecessary check_filled call from PviDeviceConnector.connect_mock - Add comprehensive test for DeviceFiller.check_filled testing both success and failure paths with mandatory and optional signals - Update test comment to clarify optional signal behavior Co-authored-by: Tom C (DLS) <[email protected]>
|
|
||
| if unfilled_mandatory: | ||
| raise RuntimeError( | ||
| f"{self._device.name}: cannot provision {unfilled} from {source}" |
There was a problem hiding this comment.
| f"{self._device.name}: cannot provision {unfilled} from {source}" | |
| f"{self._device.name}: cannot provision {unfilled_mandatory} from {source}" |
Addresses #1163