From 38d4ea2681a5cd377bd2d5bc931526e7b92ccef3 Mon Sep 17 00:00:00 2001 From: Pierre Equoy Date: Mon, 9 Dec 2024 16:35:53 +0100 Subject: [PATCH 01/10] Create a function to retrieve version and packaging information --- checkbox-ng/plainbox/__init__.py | 47 +++++++++++++++++++++++++++++++ checkbox-ng/plainbox/test_init.py | 43 ++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+) create mode 100644 checkbox-ng/plainbox/test_init.py diff --git a/checkbox-ng/plainbox/__init__.py b/checkbox-ng/plainbox/__init__.py index 3f9e090012..7db1218178 100644 --- a/checkbox-ng/plainbox/__init__.py +++ b/checkbox-ng/plainbox/__init__.py @@ -54,3 +54,50 @@ def get_version_string(): else: version_string = "{} {}".format("Checkbox", __version__) return version_string + + +def get_origin(): + """ + Return a dictionary containing information such as the version and what + packaging method is being used (Python virtual environment, Snap or + Debian). + """ + import os + import subprocess + + if os.getenv("SNAP_NAME"): + origin = { + "name": "Checkbox", + "version": __version__, + "packaging": { + "type": "snap", + "name": os.getenv("SNAP_NAME"), + "version": os.getenv("SNAP_VERSION"), + "revision": os.getenv("SNAP_REVISION"), + }, + } + elif os.getenv("VIRTUAL_ENV"): + origin = { + "name": "Checkbox", + "version": __version__, + "packaging": { + "type": "source", + "version": __version__, + }, + } + else: + dpkg_info = subprocess.check_output( + ["dpkg", "-S", __path__[0]], universal_newlines=True + ) + # 'python3-checkbox-ng: /usr/lib/python3/dist-packages/plainbox\n' + package_name = dpkg_info.split(":")[0] + origin = { + "name": "Checkbox", + "version": __version__, + "packaging": { + "type": "debian", + "name": package_name, + "version": __version__, + }, + } + return origin diff --git a/checkbox-ng/plainbox/test_init.py b/checkbox-ng/plainbox/test_init.py new file mode 100644 index 0000000000..4167df6cd4 --- /dev/null +++ b/checkbox-ng/plainbox/test_init.py @@ -0,0 +1,43 @@ +# This file is part of Checkbox. +# +# Copyright 2024 Canonical Ltd. +# Written by: +# Pierre Equoy +# +# Checkbox 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. +# +# Checkbox 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 Checkbox. If not, see . + +from unittest import TestCase, mock + +import os +import plainbox + + +class PlainboxInitTests(TestCase): + @mock.patch.dict(os.environ, {"VIRTUAL_ENV": "test"}) + def test_get_origin_venv(self): + origin = plainbox.get_origin() + self.assertEqual(origin["packaging"]["type"], "source") + + @mock.patch.dict(os.environ, {"SNAP_NAME": "test"}) + def test_get_origin_snap(self): + origin = plainbox.get_origin() + self.assertEqual(origin["packaging"]["type"], "snap") + + @mock.patch.dict(os.environ, {}, clear=True) + @mock.patch("subprocess.check_output") + def test_get_origin_debian(self, mock_sp_check_output): + mock_sp_check_output.return_value = ( + "python3-checkbox-ng: /usr/lib/python3/dist-packages/plainbox\n" + ) + origin = plainbox.get_origin() + self.assertEqual(origin["packaging"]["type"], "debian") From 1022d869452127c7ac07e2f7a4461392fb756a58 Mon Sep 17 00:00:00 2001 From: Pierre Equoy Date: Mon, 9 Dec 2024 16:36:23 +0100 Subject: [PATCH 02/10] Use get_origin() to store origin information in submission JSON --- checkbox-ng/plainbox/impl/exporter/jinja2.py | 7 +++---- .../plainbox/impl/providers/exporters/data/checkbox.json | 3 +-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/checkbox-ng/plainbox/impl/exporter/jinja2.py b/checkbox-ng/plainbox/impl/exporter/jinja2.py index 78081e3535..e5cd4523e2 100644 --- a/checkbox-ng/plainbox/impl/exporter/jinja2.py +++ b/checkbox-ng/plainbox/impl/exporter/jinja2.py @@ -45,7 +45,7 @@ from plainbox import get_version_string -from plainbox import __version__ as checkbox_version +from plainbox import get_origin from plainbox.abc import ISessionStateExporter from plainbox.impl.exporter import SessionStateExporterBase from plainbox.impl.result import OUTCOME_METADATA_MAP @@ -110,7 +110,6 @@ def __init__( self._client_version = client_version or get_version_string() # Remember client name self._client_name = client_name - self._checkbox_version = checkbox_version self.option_list = None self.template = None @@ -197,7 +196,7 @@ def dump_from_session_manager(self, session_manager, stream): "OUTCOME_METADATA_MAP": OUTCOME_METADATA_MAP, "client_name": self._client_name, "client_version": self._client_version, - "checkbox_version": self._checkbox_version, + "origin": get_origin(), "manager": session_manager, "app_blob": app_blob_data, "options": self.option_list, @@ -223,7 +222,7 @@ def dump_from_session_manager_list(self, session_manager_list, stream): "OUTCOME_METADATA_MAP": OUTCOME_METADATA_MAP, "client_name": self._client_name, "client_version": self._client_version, - "checkbox_version": self._checkbox_version, + "origin": get_origin(), "manager_list": session_manager_list, "app_blob": {}, "options": self.option_list, diff --git a/checkbox-ng/plainbox/impl/providers/exporters/data/checkbox.json b/checkbox-ng/plainbox/impl/providers/exporters/data/checkbox.json index 53a1673e14..9ec926080b 100644 --- a/checkbox-ng/plainbox/impl/providers/exporters/data/checkbox.json +++ b/checkbox-ng/plainbox/impl/providers/exporters/data/checkbox.json @@ -6,8 +6,7 @@ {%- set system_information = state.system_information -%} { "title": {{ state.metadata.title | jsonify | safe }}, - "client_version": {{ client_version | jsonify | safe }}, - "checkbox_version": {{ checkbox_version | jsonify | safe }}, + "origin": {{ origin | jsonify | safe }}, {%- if "testplan_id" in app_blob %} {%- if app_blob['testplan_id'] %} "testplan_id": {{ app_blob['testplan_id'] | jsonify | safe }}, From 0d527adec93c62f0139a743df535341628f98983 Mon Sep 17 00:00:00 2001 From: Pierre Equoy Date: Tue, 10 Dec 2024 10:41:19 +0100 Subject: [PATCH 03/10] Switch version 2020-12 of schema (from draft-06) This version is more up-to-date and allows to use conditions (if..then) --- submission-schema/schema.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/submission-schema/schema.json b/submission-schema/schema.json index baaa98aa0e..4a2ade021d 100644 --- a/submission-schema/schema.json +++ b/submission-schema/schema.json @@ -1,5 +1,5 @@ { - "$schema": "http://json-schema.org/draft-06/schema#", + "$schema": "https://json-schema.org/draft/2020-12/schema", "$id": "https://certification.canonical.com/checkbox-submission.json", "description": "Output format for the Checkbox test framework (see more info at https://checkbox.readthedocs.io)", "$ref": "#/definitions/Submission", From 19243baf2e6feac90e72f6226fe09ae924737360 Mon Sep 17 00:00:00 2001 From: Pierre Equoy Date: Tue, 10 Dec 2024 10:41:47 +0100 Subject: [PATCH 04/10] Remove client_version and checkbox_version and add origin into the JSON schema --- submission-schema/schema.json | 47 ++++++++++++++++++++++++++++++----- 1 file changed, 41 insertions(+), 6 deletions(-) diff --git a/submission-schema/schema.json b/submission-schema/schema.json index 4a2ade021d..3ce787b640 100644 --- a/submission-schema/schema.json +++ b/submission-schema/schema.json @@ -11,12 +11,47 @@ "title": { "type": "string" }, - "client_version": { - "type": "string" - }, - "checkbox_version": { - "type": "string" - }, + "origin": { + "type": "object", + "required": [ + "name", + "version", + "packaging" + ], + "properties": { + "name": { + "type": "string", + "description": "Name of the application used to run the test plan ('Checkbox')" + }, + "version": { + "type": "string", + "description": "Version of Checkbox used to run the test plan." + }, + "packaging": { + "type": "object", + "properties": { + "type": { + "type": "string", + "description": "Packaging type ('debian', 'snap', or 'source' if running in a Python virtual env" + }, + "name": { + "type": "string", + "description": "Name of the package used, if any" + }, + "version": { + "type": "string", + "description": "Version installed" + }, + "revision": { + "type": "string", + "description": "Revision installed (only for Snaps)" + } + }, + "if": {"properties": {"type": {"const": "snap"}}}, + "then": {"required": ["revision"]} + } + } + }, "testplan_id": { "type": "string" }, From 1167327e27f12e7c4852d14e8bf59198021c7ba6 Mon Sep 17 00:00:00 2001 From: Pierre Equoy Date: Tue, 10 Dec 2024 12:00:31 +0100 Subject: [PATCH 05/10] Refactor Jinja2SessionStateExporter to set origin at init time By setting the origin at init, it is more readable and easier to write unit tests. --- checkbox-ng/plainbox/impl/exporter/jinja2.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/checkbox-ng/plainbox/impl/exporter/jinja2.py b/checkbox-ng/plainbox/impl/exporter/jinja2.py index e5cd4523e2..336529114c 100644 --- a/checkbox-ng/plainbox/impl/exporter/jinja2.py +++ b/checkbox-ng/plainbox/impl/exporter/jinja2.py @@ -94,6 +94,7 @@ def __init__( timestamp=None, client_version=None, client_name="plainbox", + origin=None, exporter_unit=None, ): """ @@ -110,6 +111,7 @@ def __init__( self._client_version = client_version or get_version_string() # Remember client name self._client_name = client_name + self._origin = origin or get_origin() self.option_list = None self.template = None @@ -196,7 +198,7 @@ def dump_from_session_manager(self, session_manager, stream): "OUTCOME_METADATA_MAP": OUTCOME_METADATA_MAP, "client_name": self._client_name, "client_version": self._client_version, - "origin": get_origin(), + "origin": self._origin, "manager": session_manager, "app_blob": app_blob_data, "options": self.option_list, @@ -222,7 +224,7 @@ def dump_from_session_manager_list(self, session_manager_list, stream): "OUTCOME_METADATA_MAP": OUTCOME_METADATA_MAP, "client_name": self._client_name, "client_version": self._client_version, - "origin": get_origin(), + "origin": self._origin, "manager_list": session_manager_list, "app_blob": {}, "options": self.option_list, From 925fff38d497df471c223f6fe605115b336be0a1 Mon Sep 17 00:00:00 2001 From: Pierre Equoy Date: Tue, 10 Dec 2024 12:01:12 +0100 Subject: [PATCH 06/10] Update unit tests to use the new origin information --- .../plainbox/impl/exporter/test_html.py | 14 ++++++++ .../plainbox/impl/exporter/test_jinja2.py | 33 +++++++++++++++++-- 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/checkbox-ng/plainbox/impl/exporter/test_html.py b/checkbox-ng/plainbox/impl/exporter/test_html.py index 7cce67b74e..2b70b5fdb1 100644 --- a/checkbox-ng/plainbox/impl/exporter/test_html.py +++ b/checkbox-ng/plainbox/impl/exporter/test_html.py @@ -179,6 +179,13 @@ def test_perfect_match_without_certification_status(self): system_id="", timestamp="2012-12-21T12:00:00", client_version="Checkbox 1.0", + origin={ + "name": "Checkbox", + "version": "1.0", + "packaging": { + "type": "source" + } + }, exporter_unit=self.exporter_unit, ) stream = io.BytesIO() @@ -202,6 +209,13 @@ def test_perfect_match_with_certification_blocker(self): system_id="", timestamp="2012-12-21T12:00:00", client_version="Checkbox 1.0", + origin={ + "name": "Checkbox", + "version": "1.0", + "packaging": { + "type": "source" + } + }, exporter_unit=self.exporter_unit, ) stream = io.BytesIO() diff --git a/checkbox-ng/plainbox/impl/exporter/test_jinja2.py b/checkbox-ng/plainbox/impl/exporter/test_jinja2.py index 2c7e0bde2e..4df6b81e4a 100644 --- a/checkbox-ng/plainbox/impl/exporter/test_jinja2.py +++ b/checkbox-ng/plainbox/impl/exporter/test_jinja2.py @@ -76,7 +76,16 @@ def test_template(self): exporter_unit.option_list = () with open(pathname, "w") as f: f.write(tmpl) - exporter = Jinja2SessionStateExporter(exporter_unit=exporter_unit) + exporter = Jinja2SessionStateExporter( + origin={ + "name": "Checkbox", + "version": "1.0", + "packaging": { + "type": "source" + } + }, + exporter_unit=exporter_unit, + ) stream = BytesIO() exporter.dump_from_session_manager(self.manager_single_job, stream) expected_bytes = " fail : job name\n".encode("UTF-8") @@ -114,7 +123,16 @@ def test_validation_json(self): exporter_unit.data_dir = tmp exporter_unit.template = template_filename exporter_unit.option_list = () - exporter = Jinja2SessionStateExporter(exporter_unit=exporter_unit) + exporter = Jinja2SessionStateExporter( + origin={ + "name": "Checkbox", + "version": "1.0", + "packaging": { + "type": "source" + } + }, + exporter_unit=exporter_unit + ) stream = BytesIO() exporter.dump_from_session_manager(self.manager_single_job, stream) @@ -131,7 +149,16 @@ def test_validation_json_throws(self): exporter_unit.data_dir = tmp exporter_unit.template = template_filename exporter_unit.option_list = () - exporter = Jinja2SessionStateExporter(exporter_unit=exporter_unit) + exporter = Jinja2SessionStateExporter( + origin={ + "name": "Checkbox", + "version": "1.0", + "packaging": { + "type": "source" + } + }, + exporter_unit=exporter_unit + ) stream = BytesIO() with self.assertRaises(ExporterError): exporter.dump_from_session_manager( From 542cc9c35c438c0e0d42db70f30d6b0ade07fa46 Mon Sep 17 00:00:00 2001 From: Pierre Equoy Date: Tue, 10 Dec 2024 12:04:48 +0100 Subject: [PATCH 07/10] fix unit test + black formatting issues --- .../plainbox/impl/exporter/test_html.py | 8 ++---- .../plainbox/impl/exporter/test_jinja2.py | 25 ++++++++++--------- 2 files changed, 15 insertions(+), 18 deletions(-) diff --git a/checkbox-ng/plainbox/impl/exporter/test_html.py b/checkbox-ng/plainbox/impl/exporter/test_html.py index 2b70b5fdb1..51e8a3f9d6 100644 --- a/checkbox-ng/plainbox/impl/exporter/test_html.py +++ b/checkbox-ng/plainbox/impl/exporter/test_html.py @@ -182,9 +182,7 @@ def test_perfect_match_without_certification_status(self): origin={ "name": "Checkbox", "version": "1.0", - "packaging": { - "type": "source" - } + "packaging": {"type": "source"}, }, exporter_unit=self.exporter_unit, ) @@ -212,9 +210,7 @@ def test_perfect_match_with_certification_blocker(self): origin={ "name": "Checkbox", "version": "1.0", - "packaging": { - "type": "source" - } + "packaging": {"type": "source"}, }, exporter_unit=self.exporter_unit, ) diff --git a/checkbox-ng/plainbox/impl/exporter/test_jinja2.py b/checkbox-ng/plainbox/impl/exporter/test_jinja2.py index 4df6b81e4a..120c7f3cc8 100644 --- a/checkbox-ng/plainbox/impl/exporter/test_jinja2.py +++ b/checkbox-ng/plainbox/impl/exporter/test_jinja2.py @@ -80,9 +80,7 @@ def test_template(self): origin={ "name": "Checkbox", "version": "1.0", - "packaging": { - "type": "source" - } + "packaging": {"type": "source"}, }, exporter_unit=exporter_unit, ) @@ -104,7 +102,14 @@ def test_validation_chooses_json(self): exporter_unit.data_dir = tmp exporter_unit.template = template_filename exporter_unit.option_list = () - exporter = Jinja2SessionStateExporter(exporter_unit=exporter_unit) + exporter = Jinja2SessionStateExporter( + origin={ + "name": "Checkbox", + "version": "1.0", + "packaging": {"type": "source"}, + }, + exporter_unit=exporter_unit, + ) exporter.validate_json = mock.Mock(return_value=[]) stream = BytesIO() exporter.validate(stream) @@ -127,11 +132,9 @@ def test_validation_json(self): origin={ "name": "Checkbox", "version": "1.0", - "packaging": { - "type": "source" - } + "packaging": {"type": "source"}, }, - exporter_unit=exporter_unit + exporter_unit=exporter_unit, ) stream = BytesIO() exporter.dump_from_session_manager(self.manager_single_job, stream) @@ -153,11 +156,9 @@ def test_validation_json_throws(self): origin={ "name": "Checkbox", "version": "1.0", - "packaging": { - "type": "source" - } + "packaging": {"type": "source"}, }, - exporter_unit=exporter_unit + exporter_unit=exporter_unit, ) stream = BytesIO() with self.assertRaises(ExporterError): From 831c89555412f03584edbfe304de96765f2e8e9a Mon Sep 17 00:00:00 2001 From: Pierre Equoy Date: Tue, 10 Dec 2024 14:39:39 +0100 Subject: [PATCH 08/10] Add extra required fields for the packaging section of origin field --- submission-schema/schema.json | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/submission-schema/schema.json b/submission-schema/schema.json index 3ce787b640..955e144a79 100644 --- a/submission-schema/schema.json +++ b/submission-schema/schema.json @@ -29,6 +29,10 @@ }, "packaging": { "type": "object", + "required": [ + "type", + "version" + ], "properties": { "type": { "type": "string", From 774f73fd1cc42f01961cd8c36630a6a9f9c23856 Mon Sep 17 00:00:00 2001 From: Pierre Equoy Date: Wed, 11 Dec 2024 14:01:37 +0100 Subject: [PATCH 09/10] Add "unknown" packaging type if none of the package check worked --- checkbox-ng/plainbox/__init__.py | 38 ++++++++++++++++++++----------- checkbox-ng/plainbox/test_init.py | 9 ++++++++ 2 files changed, 34 insertions(+), 13 deletions(-) diff --git a/checkbox-ng/plainbox/__init__.py b/checkbox-ng/plainbox/__init__.py index 7db1218178..c049d37d6f 100644 --- a/checkbox-ng/plainbox/__init__.py +++ b/checkbox-ng/plainbox/__init__.py @@ -86,18 +86,30 @@ def get_origin(): }, } else: - dpkg_info = subprocess.check_output( - ["dpkg", "-S", __path__[0]], universal_newlines=True - ) - # 'python3-checkbox-ng: /usr/lib/python3/dist-packages/plainbox\n' - package_name = dpkg_info.split(":")[0] - origin = { - "name": "Checkbox", - "version": __version__, - "packaging": { - "type": "debian", - "name": package_name, + try: + dpkg_info = subprocess.check_output( + ["dpkg", "-S", __path__[0]], universal_newlines=True + ) + # 'python3-checkbox-ng: /usr/lib/python3/dist-packages/plainbox\n' + package_name = dpkg_info.split(":")[0] + origin = { + "name": "Checkbox", "version": __version__, - }, - } + "packaging": { + "type": "debian", + "name": package_name, + "version": __version__, + }, + } + # if all of the above failed and dpkg is not available on the system... + except FileNotFoundError: + origin = { + "name": "Checkbox", + "version": __version__, + "packaging": { + "type": "unknown", + "name": "unknown", + "version": "unknown", + }, + } return origin diff --git a/checkbox-ng/plainbox/test_init.py b/checkbox-ng/plainbox/test_init.py index 4167df6cd4..7429267883 100644 --- a/checkbox-ng/plainbox/test_init.py +++ b/checkbox-ng/plainbox/test_init.py @@ -19,6 +19,7 @@ from unittest import TestCase, mock import os + import plainbox @@ -41,3 +42,11 @@ def test_get_origin_debian(self, mock_sp_check_output): ) origin = plainbox.get_origin() self.assertEqual(origin["packaging"]["type"], "debian") + self.assertEqual(origin["packaging"]["name"], "python3-checkbox-ng") + + @mock.patch.dict(os.environ, {}, clear=True) + @mock.patch("subprocess.check_output") + def test_get_origin_exception(self, mock_sp_check_output): + mock_sp_check_output.side_effect = FileNotFoundError + origin = plainbox.get_origin() + self.assertEqual(origin["packaging"]["type"], "unknown") From 77efcdcd57dfc9033462c51b06860cd5be72d335 Mon Sep 17 00:00:00 2001 From: Pierre Equoy Date: Wed, 11 Dec 2024 14:32:51 +0100 Subject: [PATCH 10/10] Handle CalledProcessError as well FileNotFoundError is raised when the dpkg command is not available, but otherwise subprocess.check_output will raise CalledProcessError, which is handled in this commit. --- checkbox-ng/plainbox/__init__.py | 2 +- checkbox-ng/plainbox/test_init.py | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/checkbox-ng/plainbox/__init__.py b/checkbox-ng/plainbox/__init__.py index c049d37d6f..b86fe4609f 100644 --- a/checkbox-ng/plainbox/__init__.py +++ b/checkbox-ng/plainbox/__init__.py @@ -102,7 +102,7 @@ def get_origin(): }, } # if all of the above failed and dpkg is not available on the system... - except FileNotFoundError: + except (FileNotFoundError, subprocess.CalledProcessError): origin = { "name": "Checkbox", "version": __version__, diff --git a/checkbox-ng/plainbox/test_init.py b/checkbox-ng/plainbox/test_init.py index 7429267883..d947808848 100644 --- a/checkbox-ng/plainbox/test_init.py +++ b/checkbox-ng/plainbox/test_init.py @@ -19,6 +19,7 @@ from unittest import TestCase, mock import os +from subprocess import CalledProcessError import plainbox @@ -50,3 +51,6 @@ def test_get_origin_exception(self, mock_sp_check_output): mock_sp_check_output.side_effect = FileNotFoundError origin = plainbox.get_origin() self.assertEqual(origin["packaging"]["type"], "unknown") + mock_sp_check_output.side_effect = CalledProcessError(1, "error") + origin = plainbox.get_origin() + self.assertEqual(origin["packaging"]["type"], "unknown")