-
Notifications
You must be signed in to change notification settings - Fork 53
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 Jobs for V4L2 Compliance (New) #1653
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1653 +/- ##
==========================================
+ Coverage 48.96% 49.10% +0.14%
==========================================
Files 371 374 +3
Lines 40296 40420 +124
Branches 6808 6834 +26
==========================================
+ Hits 19730 19848 +118
- Misses 19846 19848 +2
- Partials 720 724 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR.
I left a couple of small things and a comment regarding the templated jobs.
I think there is no advantage in having all of these jobs separated into templated jobs. Everything could be done in a single compliance job, but let me know your thoughts on that.
providers/base/units/camera/jobs.pxu
Outdated
unit: template | ||
category_id: com.canonical.plainbox::camera | ||
template-resource: camera/v4l2-ioctl_resource | ||
template-unit: job | ||
template-id: camera/v4l2-compliance_ioctl_name | ||
_template-summary: To check if {ioctl_name} works on {name} | ||
id: camera/v4l2-compliance_{ioctl_name} | ||
_summary: v4l2 compliance for {ioctl_name} on {name} | ||
plugin: shell | ||
command: | ||
v4l2_compliance_test.py --device /dev/{name} --ioctl {ioctl_name} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think we need a templated job for this?
In my case, it creates 60+ jobs. This is not inherently bad, but I don't see the point of having them as separate jobs. The output of the test is quite straightforward, and having all of these separated tests will involve more scrolling during test review, and maybe missing some other important results.
As I said, that's a minor issue if there was an advantage of having there in separate jobs, but I think a joint report for all the compliance tests in one file will be better
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used templated job for this mostly because we aren't 100% sure which ioctls are actual cert-blockers and which ones aren't. On 22.04 cameras (including USB ones that typically don't break) can fail 1 or 2 tests but the camera will appear fully functional for everyday use. Each ubuntu version also produces slightly different results for the same functioning camera. Therefore having the job templated can help us eliminate non-cert-blockers later on by filtering them out in the resource job or move them to a separate template job specifically for non-cert-blockers.
It also helps with opening/managing v4l2 bug reports since each report is related to only 1 ioctl. We don't have to read the entire jira card to see which ioctl have been fixed, which one was skipped, which one was rejected by ODM, etc.
Let me know if you think this approach makes sense. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Btw if the number of jobs is a concern maybe we could group some of the ioctls in the resource job? Instead of creating 1 job per ioctl, we could do 1 job per category given by v4l2-compliance. For example instead of 7 jobs we can have just 2 (Codec ioctl job and Buffer ioctl job)
It would look something like this
V4L2 Compliance Tests (checkbox category)
├─ Required ioctls (checkbox job, name is v4l2-compliance category)
├─ Allow for multiple opens
├─ Debug ioctls
├─ Input ioctls
├─ Output ioctls
├─ I/O Config ioctls
├─ Control ioctls
├─ Format ioctls
├─ Codec ioctls
└─ Buffer ioctls
Description
This PR implements test jobs utilizing the v4l2 compliance parser. Each camera will get a job for each ioctl request, so the total number of jobs would be (n_cameras * n_ioctl_tests).
Resolved issues
Documentation
As of right now each camera will run v4l2-compliance once for every ioctl request. To test more ioctls in 1 run, pass in more ioctl names in --ioctl.
Tests
Unit tests
Test plan tested on 18, 20 VM with camera pass through, 22&24 DUTs with USB cameras.