Skip to content

Conversation

URJala
Copy link
Collaborator

@URJala URJala commented Jan 9, 2025

Added some launch arguments and some checks to avoid connecting to the dashboard interface when using mock hardware, as that will not work. Maybe not the most elegant, but it works for now. Currently passthrough controller does not work at all with mock hardware, and is bypassed if using that in the test. test_trajectory_scaled_aborts_on_violation fails, as the hardware doesnt abort. test_set_io also fails as the controller cant verify that a pin has been set.

Copy link

codecov bot commented Jan 9, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 18.56%. Comparing base (1b121b7) to head (3201f73).
⚠️ Report is 478 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##            main    #1226       +/-   ##
==========================================
+ Coverage   3.59%   18.56%   +14.97%     
==========================================
  Files         13       33       +20     
  Lines        947     3409     +2462     
  Branches     152      421      +269     
==========================================
+ Hits          34      633      +599     
- Misses       843     2745     +1902     
+ Partials      70       31       -39     
Flag Coverage Δ
unittests 18.56% <ø> (+14.97%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@URJala URJala linked an issue Jan 10, 2025 that may be closed by this pull request
@URJala URJala force-pushed the mock_hardware_tests branch from 1c6e90d to 44cfe0b Compare March 14, 2025 13:57
@URJala URJala force-pushed the mock_hardware_tests branch from 0d259f8 to 1727bd1 Compare April 8, 2025 12:46
@URJala URJala marked this pull request as ready for review April 8, 2025 14:16
@URJala URJala force-pushed the mock_hardware_tests branch from db57a95 to 72ac7ce Compare April 8, 2025 14:20
Copy link
Member

@urfeex urfeex left a comment

Choose a reason for hiding this comment

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

I think it is a good idea to split up the robot_driver test into individual tests.

However, I think it would be beneficial to be able to specify mock hardware outside of the test parametrization. This way, we can run the mock hardware tests also when the integration tests flag is not set. This would effectively allow running some tests on the buildfarm, as well.

@URJala URJala force-pushed the mock_hardware_tests branch from ac702c4 to c488e21 Compare April 22, 2025 10:57
@URJala URJala force-pushed the mock_hardware_tests branch from 8c02090 to 3cc8a2c Compare July 21, 2025 10:50
@URJala URJala requested a review from urfeex July 21, 2025 12:19
Copy link
Member

@urfeex urfeex left a comment

Choose a reason for hiding this comment

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

This seems fine so far and I think with renaming all the tests I would be fine with merging this. There is more potential for refactoring things like the initialization, but for this PR I think it's fine to leave it like that.

Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this test also be able to run on mock hardware? I see, it doesn't, but I don't see a conceptual reason why it shouldn't.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There is no real reason that it shouldn't, but with the way the controller sets io's, it apparently cannot verify if a pin was set, and fails the test because the pins indeed end up at the wrong value. The controller also prints a little warning that this can happen with mock hardware, so I don't think it should be tested, as it doesn't behave correctly.

URJala added 17 commits July 30, 2025 10:33
Added some launch arguments and some checks to avoid connecting to the dashboard interface when using mock hardware, as that will not work. Maybe not the most elegant, but it works for now.
Currently passthrough controller does not work at all with mock hardware, and is bypassed if using that in the test.
test_trajectory_scaled_aborts_on_violation fails, as the hardware doesnt abort.
test_set_io also fails as the controller cant verify that a pin has been set.
And into separate test cases.
Moved timout for trajectory execution to common file
@URJala URJala force-pushed the mock_hardware_tests branch from 7eaf5a8 to 3201f73 Compare July 30, 2025 10:50
@URJala URJala requested a review from urfeex August 4, 2025 08:28
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.

Add test using mock_hardware
2 participants