Test Jobs for V4L2 Compliance (New) #1653

tomli380576 · 2024-12-12T07:18:15Z

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.

codecov · 2024-12-12T07:19:35Z

Codecov Report

Attention: Patch coverage is 92.30769% with 3 lines in your changes missing coverage. Please review.

Project coverage is 49.00%. Comparing base (aac167f) to head (38f9e68).
Report is 32 commits behind head on main.

Files with missing lines	Patch %	Lines
providers/base/bin/v4l2_compliance_test.py	91.66%	1 Missing and 1 partial ⚠️
providers/resource/bin/v4l2_ioctl_resource.py	93.33%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1653      +/-   ##
==========================================
+ Coverage   48.96%   49.00%   +0.04%     
==========================================
  Files         371      373       +2     
  Lines       40296    40335      +39     
  Branches     6808     6816       +8     
==========================================
+ Hits        19730    19766      +36     
- Misses      19846    19847       +1     
- Partials      720      722       +2

Flag	Coverage Δ
provider-base	`24.90% <91.66%> (+0.10%)`	⬆️
provider-resource	`35.88% <93.33%> (+0.88%)`	⬆️

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.

providers/resource/bin/v4l2_ioctl_resource.py

fernando79513

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.

fernando79513 · 2025-02-05T11:32:14Z

providers/base/bin/v4l2_compliance_test.py

+    parser.add_argument(
+        "--treat-unsupported-as-fail",
+        action="store_true",
+        help="If specified, and if any of the ioctls are in unsupported, "
+        "they are treated as fail and will fail the test case",
+        default=False,
+    )


Are we using this flag anywhere?
We are parsing it but it's not used in the main function.

fernando79513 · 2025-02-05T11:40:04Z

providers/base/bin/v4l2_compliance_test.py

+    return_code = 0
+
+    if "VIDIOC_QUERYCAP" in details["failed"]:
+        return_code = 1
+    for ioctl_request in args.ioctl:
+        if ioctl_request in details["failed"]:
+            print(ioctl_request, "failed the test", file=sys.stderr)
+            return_code = 1
+
+    if return_code == 0:
+        print(args.ioctl, "are all supported")
+
+    return return_code


There is no need to handle the return codes. You could just raise a SystemExit if the test fails.
If you want to gather the information of all the failed requests before raising the raising the SystemExit you can do it, but there is no need to handle it in the "main guard"

fernando79513 · 2025-02-05T12:47:50Z

providers/base/bin/v4l2_compliance_test.py

@@ -0,0 +1,74 @@
+#! /usr/bin/python3


Use /usr/bin/env python3 so we don't force using the python3 interpreter under bin
Please also add the checkbox header for all the binaries. You can also include it for the tests, but we are not enforcing this that often

Suggested change

#! /usr/bin/python3

#!/usr/bin/env python3

# Copyright 2025 Canonical Ltd.

# Written by:

# Zhongning Li <zhongning.li@canonical.com>

#

# This program is free software: you can redistribute it and/or modify

# it under the terms of the GNU General Public License version 3,

# as published by the Free Software Foundation.

#

# This program is distributed in the hope that it will be useful,

# but WITHOUT ANY WARRANTY; without even the implied warranty of

# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the

# GNU General Public License for more details.

#

# You should have received a copy of the GNU General Public License

# along with this program. If not, see <http://www.gnu.org/licenses/>.

fernando79513 · 2025-02-05T12:55:56Z

providers/base/units/camera/jobs.pxu

+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}
+


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

fernando79513 · 2025-02-05T14:31:52Z

providers/base/units/camera/jobs.pxu

+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}


Also, templated summaries should not include the specific name, they should be general for all the templated jobs.

tomli380576 added 12 commits September 26, 2024 11:34

feat: initial impl of v4l2-compliance test parser

14af1cc

fix: spelling

b0d7c52

fix: naming

368d1a4

style: rename

bb352e1

feat: add test name to ioctl map

8b63aa8

fix: raise an error if the device can't be opened

39858e7

feat: v4l2-compliance test case

39a777e

feat: add resource job

0ea720f

Merge branch 'main' into v4l2-compliance-job

6c1aa84

feat: test jobs for each ioctl

2b160de

feat: generate tests for each device

3b390fc

fix: remove unused

c84e80c

tomli380576 added 4 commits January 6, 2025 10:08

test: add initial unit tests

e312403

test: add unit tests

86926bd

test: unit test for the actual job

1ef08db

fix: flake8

48f7857

tomli380576 marked this pull request as ready for review January 9, 2025 06:48

tomli380576 commented Jan 9, 2025

View reviewed changes

providers/resource/bin/v4l2_ioctl_resource.py Outdated Show resolved Hide resolved

fix: consistent file name

04a451f

tomli380576 marked this pull request as draft January 17, 2025 04:32

tomli380576 added 4 commits January 17, 2025 13:24

fix: use udevadm

187c75f

test: update unittests

88b8dbc

style: cleaner condition

aa3d30c

style: formatting

38f9e68

tomli380576 marked this pull request as ready for review January 17, 2025 06:16

fernando79513 self-assigned this Feb 4, 2025

fernando79513 reviewed Feb 5, 2025

View reviewed changes

fernando79513 added the waiting-for-changes The review has been completed but the PR is waiting for changes from the author label Feb 5, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Test Jobs for V4L2 Compliance (New) #1653

Test Jobs for V4L2 Compliance (New) #1653

tomli380576 commented Dec 12, 2024 •

edited

Loading

codecov bot commented Dec 12, 2024 •

edited

Loading

fernando79513 left a comment

fernando79513 Feb 5, 2025

fernando79513 Feb 5, 2025

fernando79513 Feb 5, 2025

fernando79513 Feb 5, 2025

fernando79513 Feb 5, 2025

-#! /usr/bin/python3
+#!/usr/bin/env python3
+# Copyright 2025 Canonical Ltd.
+# Written by:
+#   Zhongning Li <zhongning.li@canonical.com>
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License version 3,
+# as published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.

Test Jobs for V4L2 Compliance (New) #1653

Are you sure you want to change the base?

Test Jobs for V4L2 Compliance (New) #1653

Conversation

tomli380576 commented Dec 12, 2024 • edited Loading

Description

Resolved issues

Documentation

Tests

codecov bot commented Dec 12, 2024 • edited Loading

Codecov Report

fernando79513 left a comment

Choose a reason for hiding this comment

fernando79513 Feb 5, 2025

Choose a reason for hiding this comment

fernando79513 Feb 5, 2025

Choose a reason for hiding this comment

fernando79513 Feb 5, 2025

Choose a reason for hiding this comment

fernando79513 Feb 5, 2025

Choose a reason for hiding this comment

fernando79513 Feb 5, 2025

Choose a reason for hiding this comment

tomli380576 commented Dec 12, 2024 •

edited

Loading

codecov bot commented Dec 12, 2024 •

edited

Loading