Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test LogReader scenarios for auto_source #32443

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

markopetkovic96
Copy link

@markopetkovic96 markopetkovic96 commented May 16, 2024

for #31223
no logs available -> should error for all query types: test fail for all query types.
rlogs not available, but qlogs are -> should error with /r, pass with /q and /a: test pass for /q, but fail for /r and /a.
rlogs only available for specific segments, but qlogs are available for the rest -> should error with /r, pass with /a: pass with /r, error with /a.
rlog not available on comma api, but is available from another source (openpilot ci bucket, commaCarSegments), rlogs should be parsed: test return comma_api, where is not available rlog.
I wrote a few tests for auto_source, just to make sure this is a good approach. As you can see, I have deviations from the expected results in several cases. In that case, is it necessary to modify the auto_source function, or is a different approach to the test needed?
For the case where rlog is not available on the comma api, the test just checks the return from auto_source. Can you explain what you mean by rlogs should be parsed? If the return from auto_source is rlog on the comma api, where is not available, rlogs should be parsed?

Copy link
Contributor

github-actions bot commented May 16, 2024

Thanks for contributing to openpilot! 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
    • the change is something we merge
    • include a route or your device' dongle ID if relevant

@markopetkovic96
Copy link
Author

@jnewb1
After converting the unittests to pytest, I created a mock HTTP server.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant