Skip to content

test: fix MockSensor do_command state tracking and add missing coverage#1188

Open
luckys00 wants to merge 5 commits intoviamrobotics:mainfrom
luckys00:fix-mock-sensor-state
Open

test: fix MockSensor do_command state tracking and add missing coverage#1188
luckys00 wants to merge 5 commits intoviamrobotics:mainfrom
luckys00:fix-mock-sensor-state

Conversation

@luckys00
Copy link
Copy Markdown

@luckys00 luckys00 commented Apr 3, 2026

What this PR does

Fixes an inconsistency in the MockSensor class where do_command was dropping state parameters (timeout and extra), and adds the missing test coverage for do_command across the Sensor, Service, and Client test suites.

Why is it needed?

While get_readings and get_geometries properly save self.extra and self.timeout to the mock's state, the do_command implementation was bypassing this and simply returning the command. This makes it impossible to assert if custom commands with timeouts were actually processed correctly by the mock.

Additionally, this PR adds proper test coverage for do_command in test_sensor.py to ensure the timeout parameter successfully travels through the gRPC channel.

Changes Made:

  • Updated do_command in tests/mocks/components.py to properly track self.timeout and self.extra.
  • Added missing do_command tests in TestSensor, TestService, and TestClient (asserting timeout state logic while acknowledging gRPC drops extra for this specific request).

@luckys00 luckys00 requested a review from a team as a code owner April 3, 2026 19:55
@luckys00 luckys00 requested review from njooma and stuqdog April 3, 2026 19:55
@viambot
Copy link
Copy Markdown
Member

viambot commented Apr 3, 2026

👋 Thanks for requesting a review from the team!

We aim to review PRs within one business day. If this is urgent
or blocking you, please reach out in #team-sdk and
we'll prioritize it.

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Apr 4, 2026

CLA assistant check
All committers have signed the CLA.

Copy link
Copy Markdown
Member

@njooma njooma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! Thanks for contributing! We are actually in the process of updating all our tests. If you want to take a look at how the test_camera.py tests are now implemented, we are moving towards that for all our resources.

If you like, you can update the sensor tests to use that framework!

@luckys00
Copy link
Copy Markdown
Author

luckys00 commented Apr 7, 2026

Thanks for the review and the heads-up, @njooma ! It's great to hear about the new testing framework. I'll take a look at how test_camera.py is implemented and update the do_command tests for the Sensor components to match that new structure. Will push the changes shortly!

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.

4 participants