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 Jobs for V4L2 Compliance (New) #1653

Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
14af1cc
feat: initial impl of v4l2-compliance test parser
tomli380576 Sep 26, 2024
b0d7c52
fix: spelling
tomli380576 Sep 26, 2024
368d1a4
fix: naming
tomli380576 Oct 2, 2024
bb352e1
style: rename
tomli380576 Oct 3, 2024
8b63aa8
feat: add test name to ioctl map
tomli380576 Oct 29, 2024
39858e7
fix: raise an error if the device can't be opened
tomli380576 Oct 29, 2024
39a777e
feat: v4l2-compliance test case
tomli380576 Oct 30, 2024
0ea720f
feat: add resource job
tomli380576 Dec 11, 2024
6c1aa84
Merge branch 'main' into v4l2-compliance-job
tomli380576 Dec 12, 2024
2b160de
feat: test jobs for each ioctl
tomli380576 Dec 12, 2024
3b390fc
feat: generate tests for each device
tomli380576 Dec 12, 2024
c84e80c
fix: remove unused
tomli380576 Dec 12, 2024
e312403
test: add initial unit tests
tomli380576 Jan 6, 2025
86926bd
test: add unit tests
tomli380576 Jan 9, 2025
1ef08db
test: unit test for the actual job
tomli380576 Jan 9, 2025
48f7857
fix: flake8
tomli380576 Jan 9, 2025
04a451f
fix: consistent file name
tomli380576 Jan 9, 2025
187c75f
fix: use udevadm
tomli380576 Jan 17, 2025
88b8dbc
test: update unittests
tomli380576 Jan 17, 2025
aa3d30c
style: cleaner condition
tomli380576 Jan 17, 2025
38f9e68
style: formatting
tomli380576 Jan 17, 2025
b1c73f8
fix: apply Fernando's suggestions
tomli380576 Feb 24, 2025
8f7b6b7
test: update unit tests
tomli380576 Feb 24, 2025
943532c
fix (job.pxu): apply Fernando's suggestion
tomli380576 Feb 24, 2025
08b75fe
test: increase coverage
tomli380576 Feb 24, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 74 additions & 0 deletions providers/base/bin/v4l2_compliance_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
#! /usr/bin/python3

import argparse
import sys
from checkbox_support.parsers.v4l2_compliance import parse_v4l2_compliance

"""
class Input:
ioctl: T.List[str]
device: str
"""


def parse_args():
parser = argparse.ArgumentParser()
parser.add_argument(
"--device",
help="The device to test, if not specified, "
"we will let v4l2-compliance infer the default device "
"(usually /dev/video0)",
type=str,
)
parser.add_argument(
"--ioctl",
nargs="+",
help=(
"List of ioctl requests. "
"If any of them is listed under FAIL in v4l2-compliance, "
"the entire test cases fails. "
"ioctl requests should start with VIDIOC_, "
"for example VIDIOC_ENUM_FMT "
"NOTE: VIDIOC_QUERYCAP is always required"
),
default=["VIDIOC_QUERYCAP"],
)
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,
)
return parser.parse_args() # type: ignore


def main():
args = parse_args()
print(
"Testing if all of the following ioctl requests",
args.ioctl,
"are supported on",
args.device or "/dev/video0",
)

_, details = parse_v4l2_compliance(args.device)

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")

Check warning on line 67 in providers/base/bin/v4l2_compliance_test.py

View check run for this annotation

Codecov / codecov/patch

providers/base/bin/v4l2_compliance_test.py#L67

Added line #L67 was not covered by tests

return return_code


if __name__ == "__main__":
return_code = main()
exit(return_code)
116 changes: 116 additions & 0 deletions providers/base/tests/test_v4l2_compliance_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
import unittest as ut
from unittest.mock import MagicMock, call, patch
from v4l2_compliance_test import main as main_under_test
from shlex import split as sh_split


class TestV4L2ComplianceTest(ut.TestCase):

@patch("v4l2_compliance_test.parse_v4l2_compliance")
def test_failed_query_cap(self, mock_parser: MagicMock):
with patch(
"sys.argv",
sh_split("v4l2_compliance_test.py --ioctl VIDIOC_QUERYCAP"),
):
# query cap failure should always fail the test case
mock_parser.return_value = (
{},
{
"succeeded": [],
"failed": ["VIDIOC_QUERYCAP"],
"not_supported": [],
},
)

self.assertEqual(main_under_test(), 1)

with patch(
"sys.argv",
sh_split("v4l2_compliance_test.py --ioctl VIDIOC_ENUM_FMT"),
):
mock_parser.return_value = (
{},
{
"succeeded": [],
"failed": ["VIDIOC_QUERYCAP", "VIDIOC_ENUM_FMT"],
"not_supported": [],
},
)
# also fail even if query cap is not listed in the args
self.assertEqual(main_under_test(), 1)

@patch(
"sys.argv",
sh_split(
"v4l2_compliance_test.py "
+ "--ioctl VIDIOC_ENUM_FMT VIDIOC_QUERYCTRL --device /dev/video1"
),
)
@patch("sys.stderr")
@patch("builtins.print")
@patch("v4l2_compliance_test.parse_v4l2_compliance")
def test_report_correct_failures(
self,
mock_parser: MagicMock,
mock_print: MagicMock,
mock_stderr: MagicMock,
):
mock_parser.return_value = (
{},
{
"succeeded": [],
"failed": ["VIDIOC_ENUM_FMT"],
"not_supported": [],
},
)

self.assertEqual(main_under_test(), 1)
mock_print.assert_called_with(
"VIDIOC_ENUM_FMT", "failed the test", file=mock_stderr
)

mock_print.reset_mock()
mock_parser.return_value = (
{},
{
"succeeded": [],
"failed": ["VIDIOC_QUERYCTRL"],
"not_supported": [],
},
)

self.assertEqual(main_under_test(), 1)
mock_print.assert_called_with(
"VIDIOC_QUERYCTRL", "failed the test", file=mock_stderr
)

mock_print.reset_mock()
mock_parser.return_value = (
{},
{
"succeeded": [],
"failed": ["VIDIOC_QUERYCTRL", "VIDIOC_ENUM_FMT"],
"not_supported": [],
},
)

self.assertEqual(main_under_test(), 1)
mock_print.assert_has_calls(
[
call(
"Testing if all of the following ioctl requests",
[
"VIDIOC_ENUM_FMT",
"VIDIOC_QUERYCTRL",
],
"are supported on",
"/dev/video1",
),
call("VIDIOC_ENUM_FMT", "failed the test", file=mock_stderr),
call("VIDIOC_QUERYCTRL", "failed the test", file=mock_stderr),
]
)


if __name__ == "__main__":
ut.main()
21 changes: 21 additions & 0 deletions providers/base/units/camera/jobs.pxu
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,27 @@ command:
udev_resource.py -f MMAL | grep "category: MMAL"
_summary: Detect presence of a MMAL camera.

id: camera/v4l2-ioctl_resource
_summary: Generates ioctl names for v4l2 compliance test
_description: Generates ioctl names for v4l2 compliance test
estimated_duration: 0.02
category_id: com.canonical.plainbox::camera
plugin: resource
command:
v4l2_ioctl_resource.py

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}

Copy link
Collaborator

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

Copy link
Contributor Author

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!

Copy link
Contributor Author

@tomli380576 tomli380576 Feb 26, 2025

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)
image

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

unit: template
template-resource: device
template-filter: device.category == 'CAPTURE' and device.name != ''
Expand Down
2 changes: 2 additions & 0 deletions providers/base/units/camera/test-plan.pxu
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,13 @@ _description: Camera tests (automated)
include:
camera/detect certification-status=blocker
camera/multiple-resolution-images_.* certification-status=blocker
camera/v4l2-compliance_.* certification-status=blocker
camera/multiple-resolution-images-attachment_.* certification-status=non-blocker
camera/camera-quality_.* certification-status=non-blocker
camera/camera-quality-image_.* certification-status=non-blocker
bootstrap_include:
device
camera/v4l2-ioctl_resource

id: camera-cert-blockers
unit: test plan
Expand Down
23 changes: 23 additions & 0 deletions providers/resource/bin/v4l2_ioctl_resource.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
#! /usr/bin/python3

from checkbox_support.parsers.v4l2_compliance import TEST_NAME_TO_IOCTL_MAP
import subprocess as sp


def main():
udev_out = sp.check_output(
"udev_resource.py -f CAPTURE", universal_newlines=True, shell=True
)
lines = udev_out.splitlines()

for line in lines:
if line.startswith("name:"):
for ioctl_names in TEST_NAME_TO_IOCTL_MAP.values():
for name in ioctl_names:
print(line)
print("ioctl_name: {}".format(name))
print() # empty line to mark end of list item


if __name__ == "__main__":
main()
55 changes: 55 additions & 0 deletions providers/resource/tests/test_v4l2_compliance_resource.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import unittest as ut
from unittest.mock import MagicMock, patch, call
from v4l2_ioctl_resource import main as main_under_test
from checkbox_support.parsers.v4l2_compliance import TEST_NAME_TO_IOCTL_MAP


class TestV4L2IoctlResource(ut.TestCase):
@patch("v4l2_ioctl_resource.sp.check_output")
def test_all_ioctl_names_are_generated(self, mock_check_output: MagicMock):
mock_check_output.return_value = "\n".join(
[
"path: /devices/pci0000:00/0000:00:14.0/usb3/"
+ "3-9/3-9:1.0/video4linux/video0",
"name: video0",
"bus: video4linux",
"category: CAPTURE",
"driver: uvcvideo",
"product_id: 22782",
"vendor_id: 3034",
"product: Integrated_Webcam_HD: Integrate",
"vendor: CN0PW36V8LG009BQA0YWA00",
"product_slug: Integrated_Webcam_HD__Integrate",
"vendor_slug: CN0PW36V8LG009BQA0YWA00",
"",
"path: /devices/pci0000:00/0000:00:14.0/usb3/"
+ "3-9/3-9:1.0/video4linux/video1",
"name: video1",
"bus: video4linux",
"category: CAPTURE",
"driver: uvcvideo",
"product_id: 2312312",
"vendor_id: 12213",
"product: Integrated_Webcam_HD: Integrate",
"vendor: CN0PW36V3444438LG009BQA0YWA00",
"product_slug: Integrated_Webcam_HD__Integrate",
"vendor_slug: CN0PW36V8LG009BQA0YWA00",
]
)

with patch("builtins.print") as mock_print:
main_under_test()
expected_calls = []
for name in "video0", "video1":
for ioctl_names in TEST_NAME_TO_IOCTL_MAP.values():
for ioctl_name in ioctl_names:
expected_calls.append(call("name: {}".format(name)))
expected_calls.append(
call("ioctl_name: {}".format(ioctl_name))
)
expected_calls.append(call())
mock_print.assert_has_calls(expected_calls)


if __name__ == "__main__":
ut.main()
Loading