mazda: fix the CX-9 model tests and run the fork's tests in CI - #9
Merged
Conversation
MazdaPlatformConfig's default dbc_dict gained Bus.radar for the CX-5 2022 radar track work, which handed every Mazda a radar bus and left radarUnavailable False on all of them. The 2016-20 CX-9 is the one platform that never puts the 0x361-0x366 tracks on bus 0, so its RadarInterface built a parser that never went valid and test_radar_interface failed on every segment of route 10b5a4b380434151. Give that platform its own dbc_dict without the radar bus. Verified against all six Mazda platforms on segments 0-2: only the CX-9 lacks the tracks. Also declare CAM_PEDESTRIAN in the cam parser at nan frequency, next to the three siblings already there. It is read through vl, so it was registering lazily with liveness checks on, and the 2016-20 CX-9 camera never sends it: canValid stayed false for a whole route (5745 frames on segment 0).
opendbc has no pytest: the suite runs under unittest-parallel, the safety tests under unittest discover, and pytest appears in neither pyproject.toml nor uv.lock. These five files were the only ones importing it, so discovery raised ModuleNotFoundError on each and ./test.sh failed on both runners. The 146 tests had never executed here; they only ever ran in the parent openpilot repo, whose venv has pytest. Convert them in place: fixtures become module-level factories, parametrize becomes a class-level case table walked with subTest, and approx becomes assertAlmostEqual or a small _Approx helper where the tolerance is compared inside an expression. Case coverage is unchanged at 166. TestStandstillHold and TestAdvertisedLead had a run() helper, which shadows TestCase.run and would have swallowed every test in those classes; it is drive() now. Discovery goes from 9529 tests with 5 errors to 9656 with none.
There was a problem hiding this comment.
Thanks for contributing to opendbc! In order for us to review your PR as quickly as possible, check the following:
- Convert your PR to a draft unless it's ready to review
- Read the contributing docs
- Before marking as "ready for review", ensure:
- the goal is clearly stated in the description
- all the tests are passing
- include a route or your device' dongle ID if relevant
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three failures that predate #8 and fail on every branch.
test models: MAZDA_CX9 radar and canValidMazdaPlatformConfig's defaultdbc_dictgainedBus.radarfor the CX-5 2022 track work, so every Mazda claimed a radar andradarUnavailablewas False everywhere. The 2016-20 CX-9 is the one platform that never puts the 0x361-0x366 tracks on bus 0, soRadarInterfacebuilt a parser that never went valid.Checked all six Mazda platforms against segments 0-2: only the CX-9 lacks the tracks, so it gets its own
dbc_dictwithout the radar bus.The
can_invalid_cntfailure was separate and only reproduced on segment 0, which is the segment CI happens to pick.CAM_PEDESTRIANis read throughvl, so it registered lazily with liveness checks on while its three siblings were already declared atnanfor exactly that reason. The 2016-20 CX-9 camera never sends it, socanValidwas false for the whole route (5745 frames)../test.sh: the fork's tests never ranopendbc has no pytest anywhere - the suite runs under
unittest-parallel, safety underunittest discover, and pytest is in neitherpyproject.tomlnoruv.lock. These five files were the only ones importing it, so discovery raisedModuleNotFoundErroron each. The 146 tests have never executed in this repo; they only ran in the parent openpilot repo, whose venv has pytest.Converted in place rather than adding pytest, which would put a dependency upstream does not have into files upstream owns. Fixtures become module-level factories,
parametrizebecomes a class-level case table walked withsubTest, andapproxbecomesassertAlmostEqualor a small_Approxhelper where the tolerance is compared inside an expression. Case coverage is unchanged at 166.TestStandstillHoldandTestAdvertisedLeadhad arun()helper that shadowsTestCase.run; it isdrive()now.Discovery goes from 9529 tests with 5 errors to 9656 with none.
Not addressed
The
Safety mutation testsfailure is flaky, not a real regression: it passed on 8ec52e0 and failed on f8c21af, and the only diff between them is a comment rewrite. The four survivors are inlateral.handlongitudinal.h, which #8 never touched.Verified locally
test_car_interface+test_radar_interface, segments 0-2unittest-parallel -j4: 9656 tests, OK