From c948e4182b9557f2befeb6210a055d0aa1c2df21 Mon Sep 17 00:00:00 2001 From: James Falcon Date: Wed, 14 Feb 2024 16:25:11 -0600 Subject: [PATCH] fix: unpin jsonschema and update tests (#4882) In 034a5cd , we pinned jsonschema version due to failing tests. Test failures were due to jsonschema library changing error messages. This commit unpins the version and updates tests accordingly. Fixes GH-4783 Co-authored-by: dermotbradley --- .../unittests/config/test_cc_apk_configure.py | 5 +++-- .../unittests/config/test_cc_apt_configure.py | 14 ++++++------- tests/unittests/config/test_cc_bootcmd.py | 17 +++++++++++----- tests/unittests/config/test_cc_ca_certs.py | 10 +++++++--- tests/unittests/config/test_cc_chef.py | 5 +++-- tests/unittests/config/test_cc_lxd.py | 2 +- tests/unittests/config/test_cc_mounts.py | 10 ++++++++-- .../test_cc_package_update_upgrade_install.py | 3 ++- tests/unittests/config/test_cc_runcmd.py | 3 ++- .../unittests/config/test_cc_set_passwords.py | 9 +++++++-- tests/unittests/config/test_cc_snap.py | 20 +++++++++++++------ tests/unittests/config/test_cc_write_files.py | 6 +++++- .../unittests/config/test_cc_yum_add_repo.py | 2 +- tests/unittests/helpers.py | 8 ++++++++ tests/unittests/test_merging.py | 2 +- 15 files changed, 81 insertions(+), 35 deletions(-) diff --git a/tests/unittests/config/test_cc_apk_configure.py b/tests/unittests/config/test_cc_apk_configure.py index 8854670..e9533a1 100644 --- a/tests/unittests/config/test_cc_apk_configure.py +++ b/tests/unittests/config/test_cc_apk_configure.py @@ -18,6 +18,7 @@ from cloudinit.config.schema import ( validate_cloudconfig_schema, ) from tests.unittests.helpers import ( + SCHEMA_EMPTY_ERROR, FilesystemMockingTestCase, mock, skipUnlessJsonSchema, @@ -355,7 +356,7 @@ class TestApkConfigureSchema: ( {"apk_repos": {"alpine_repo": {}}}, "apk_repos.alpine_repo: 'version' is a required property," - " apk_repos.alpine_repo: {} does not have enough properties", + f" apk_repos.alpine_repo: {{}} {SCHEMA_EMPTY_ERROR}", ), ( {"apk_repos": {"alpine_repo": True}}, @@ -368,7 +369,7 @@ class TestApkConfigureSchema: ), ( {"apk_repos": {}}, - "apk_repos: {} does not have enough properties", + f"apk_repos: {{}} {SCHEMA_EMPTY_ERROR}", ), ( {"apk_repos": {"local_repo_base_url": None}}, diff --git a/tests/unittests/config/test_cc_apt_configure.py b/tests/unittests/config/test_cc_apt_configure.py index 18e6663..4fff316 100644 --- a/tests/unittests/config/test_cc_apt_configure.py +++ b/tests/unittests/config/test_cc_apt_configure.py @@ -12,7 +12,7 @@ from cloudinit.config.schema import ( get_schema, validate_cloudconfig_schema, ) -from tests.unittests.helpers import skipUnlessJsonSchema +from tests.unittests.helpers import SCHEMA_EMPTY_ERROR, skipUnlessJsonSchema from tests.unittests.util import get_cloud @@ -34,7 +34,7 @@ class TestAPTConfigureSchema: " ('boguskey' was unexpected)" ), ), - ({"apt": {}}, "apt: {} does not have enough properties"), + ({"apt": {}}, f"apt: {{}} {SCHEMA_EMPTY_ERROR}"), ( {"apt": {"preserve_sources_list": 1}}, "apt.preserve_sources_list: 1 is not of type 'boolean'", @@ -45,7 +45,7 @@ class TestAPTConfigureSchema: ), ( {"apt": {"disable_suites": []}}, - re.escape("apt.disable_suites: [] is too short"), + re.escape("apt.disable_suites: [] ") + SCHEMA_EMPTY_ERROR, ), ( {"apt": {"disable_suites": [1]}}, @@ -65,7 +65,7 @@ class TestAPTConfigureSchema: ), ( {"apt": {"primary": []}}, - re.escape("apt.primary: [] is too short"), + re.escape("apt.primary: [] ") + SCHEMA_EMPTY_ERROR, ), ( {"apt": {"primary": ["nonobj"]}}, @@ -102,7 +102,7 @@ class TestAPTConfigureSchema: ), ( {"apt": {"primary": [{"arches": ["amd64"], "search": []}]}}, - re.escape("apt.primary.0.search: [] is too short"), + re.escape("apt.primary.0.search: [] ") + SCHEMA_EMPTY_ERROR, ), ( { @@ -134,7 +134,7 @@ class TestAPTConfigureSchema: ), ( {"apt": {"debconf_selections": {}}}, - "apt.debconf_selections: {} does not have enough properties", + f"apt.debconf_selections: {{}} {SCHEMA_EMPTY_ERROR}", ), ( {"apt": {"sources_list": True}}, @@ -170,7 +170,7 @@ class TestAPTConfigureSchema: ), ( {"apt": {"sources": {"opaquekey": {}}}}, - "apt.sources.opaquekey: {} does not have enough properties", + f"apt.sources.opaquekey: {{}} {SCHEMA_EMPTY_ERROR}", ), ( {"apt": {"sources": {"opaquekey": {"boguskey": True}}}}, diff --git a/tests/unittests/config/test_cc_bootcmd.py b/tests/unittests/config/test_cc_bootcmd.py index f6772df..b938732 100644 --- a/tests/unittests/config/test_cc_bootcmd.py +++ b/tests/unittests/config/test_cc_bootcmd.py @@ -11,7 +11,12 @@ from cloudinit.config.schema import ( get_schema, validate_cloudconfig_schema, ) -from tests.unittests.helpers import CiTestCase, mock, skipUnlessJsonSchema +from tests.unittests.helpers import ( + SCHEMA_EMPTY_ERROR, + CiTestCase, + mock, + skipUnlessJsonSchema, +) from tests.unittests.util import get_cloud @@ -128,12 +133,14 @@ class TestBootCMDSchema: "Cloud config schema errors: bootcmd: 1 is not of type" " 'array'", ), - ({"bootcmd": []}, re.escape("bootcmd: [] is too short")), ( {"bootcmd": []}, - re.escape( - "Cloud config schema errors: bootcmd: [] is too short" - ), + re.escape("bootcmd: [] ") + SCHEMA_EMPTY_ERROR, + ), + ( + {"bootcmd": []}, + re.escape("Cloud config schema errors: bootcmd: [] ") + + SCHEMA_EMPTY_ERROR, ), ( { diff --git a/tests/unittests/config/test_cc_ca_certs.py b/tests/unittests/config/test_cc_ca_certs.py index b93fda7..cb5712b 100644 --- a/tests/unittests/config/test_cc_ca_certs.py +++ b/tests/unittests/config/test_cc_ca_certs.py @@ -17,7 +17,11 @@ from cloudinit.config.schema import ( get_schema, validate_cloudconfig_schema, ) -from tests.unittests.helpers import TestCase, skipUnlessJsonSchema +from tests.unittests.helpers import ( + SCHEMA_EMPTY_ERROR, + TestCase, + skipUnlessJsonSchema, +) from tests.unittests.util import get_cloud @@ -398,7 +402,7 @@ class TestCACertsSchema: ), ( {"ca_certs": {}}, - re.escape("ca_certs: {} does not have enough properties"), + re.escape("ca_certs: {} ") + SCHEMA_EMPTY_ERROR, ), ( {"ca_certs": {"boguskey": 1}}, @@ -417,7 +421,7 @@ class TestCACertsSchema: ), ( {"ca_certs": {"trusted": []}}, - re.escape("ca_certs.trusted: [] is too short"), + re.escape("ca_certs.trusted: [] ") + SCHEMA_EMPTY_ERROR, ), ), ) diff --git a/tests/unittests/config/test_cc_chef.py b/tests/unittests/config/test_cc_chef.py index 9d8ba1f..6fad6a7 100644 --- a/tests/unittests/config/test_cc_chef.py +++ b/tests/unittests/config/test_cc_chef.py @@ -15,6 +15,7 @@ from cloudinit.config.schema import ( validate_cloudconfig_schema, ) from tests.unittests.helpers import ( + SCHEMA_EMPTY_ERROR, FilesystemMockingTestCase, ResponsesTestCase, cloud_init_project_dir, @@ -306,7 +307,7 @@ class TestBootCMDSchema: ), ( {"chef": {}}, - re.escape(" chef: {} does not have enough properties"), + re.escape(" chef: {} ") + SCHEMA_EMPTY_ERROR, ), ( {"chef": {"boguskey": True}}, @@ -321,7 +322,7 @@ class TestBootCMDSchema: ), ( {"chef": {"directories": []}}, - re.escape("chef.directories: [] is too short"), + re.escape("chef.directories: [] ") + SCHEMA_EMPTY_ERROR, ), ( {"chef": {"directories": [1]}}, diff --git a/tests/unittests/config/test_cc_lxd.py b/tests/unittests/config/test_cc_lxd.py index 648fc33..320bfdb 100644 --- a/tests/unittests/config/test_cc_lxd.py +++ b/tests/unittests/config/test_cc_lxd.py @@ -385,7 +385,7 @@ class TestLXDSchema: # Require bridge.mode ({"lxd": {"bridge": {}}}, "bridge: 'mode' is a required property"), # Require init or bridge keys - ({"lxd": {}}, "lxd: {} does not have enough properties"), + ({"lxd": {}}, f"lxd: {{}} {t_help.SCHEMA_EMPTY_ERROR}"), # Require some non-empty preseed config of type string ({"lxd": {"preseed": {}}}, "not of type 'string'"), ({"lxd": {"preseed": ""}}, None), diff --git a/tests/unittests/config/test_cc_mounts.py b/tests/unittests/config/test_cc_mounts.py index a2dada8..4795357 100644 --- a/tests/unittests/config/test_cc_mounts.py +++ b/tests/unittests/config/test_cc_mounts.py @@ -583,9 +583,15 @@ class TestMountsSchema: "config, error_msg", [ # We expect to see one mount if provided in user-data. - ({"mounts": []}, re.escape("mounts: [] is too short")), + ( + {"mounts": []}, + re.escape("mounts: [] ") + test_helpers.SCHEMA_EMPTY_ERROR, + ), # Disallow less than 1 item per mount entry - ({"mounts": [[]]}, re.escape("mounts.0: [] is too short")), + ( + {"mounts": [[]]}, + re.escape("mounts.0: [] ") + test_helpers.SCHEMA_EMPTY_ERROR, + ), # Disallow more than 6 items per mount entry ({"mounts": [["1"] * 7]}, "mounts.0:.* is too long"), # Disallow mount_default_fields will anything other than 6 items diff --git a/tests/unittests/config/test_cc_package_update_upgrade_install.py b/tests/unittests/config/test_cc_package_update_upgrade_install.py index 9ba7f17..b3d43a1 100644 --- a/tests/unittests/config/test_cc_package_update_upgrade_install.py +++ b/tests/unittests/config/test_cc_package_update_upgrade_install.py @@ -14,6 +14,7 @@ from cloudinit.config.schema import ( from cloudinit.distros import PackageInstallerError from cloudinit.subp import SubpResult from tests.unittests.helpers import skipUnlessJsonSchema +from tests.unittests.helpers import SCHEMA_EMPTY_ERROR from tests.unittests.util import get_cloud @@ -188,7 +189,7 @@ class TestPackageUpdateUpgradeSchema: # packages list with three entries (2 required) ({"packages": ["p1", ["p2", "p3", "p4"]]}, ""), # empty packages list - ({"packages": []}, "is too short"), + ({"packages": []}, SCHEMA_EMPTY_ERROR), ( {"apt_update": False}, ( diff --git a/tests/unittests/config/test_cc_runcmd.py b/tests/unittests/config/test_cc_runcmd.py index a8636bb..f69cd9d 100644 --- a/tests/unittests/config/test_cc_runcmd.py +++ b/tests/unittests/config/test_cc_runcmd.py @@ -14,6 +14,7 @@ from cloudinit.config.schema import ( validate_cloudconfig_schema, ) from tests.unittests.helpers import ( + SCHEMA_EMPTY_ERROR, FilesystemMockingTestCase, skipUnlessJsonSchema, ) @@ -90,7 +91,7 @@ class TestRunCmdSchema: ({"runcmd": ["echo bye", "echo bye"]}, None), # Invalid schemas ({"runcmd": 1}, "1 is not of type 'array'"), - ({"runcmd": []}, r"runcmd: \[\] is too short"), + ({"runcmd": []}, rf"runcmd: \[\] {SCHEMA_EMPTY_ERROR}"), ( { "runcmd": [ diff --git a/tests/unittests/config/test_cc_set_passwords.py b/tests/unittests/config/test_cc_set_passwords.py index 1a9fcd3..ef34a8c 100644 --- a/tests/unittests/config/test_cc_set_passwords.py +++ b/tests/unittests/config/test_cc_set_passwords.py @@ -12,7 +12,11 @@ from cloudinit.config.schema import ( get_schema, validate_cloudconfig_schema, ) -from tests.unittests.helpers import does_not_raise, skipUnlessJsonSchema +from tests.unittests.helpers import ( + SCHEMA_EMPTY_ERROR, + does_not_raise, + skipUnlessJsonSchema, +) from tests.unittests.util import get_cloud MODPATH = "cloudinit.config.cc_set_passwords." @@ -718,7 +722,8 @@ class TestSetPasswordsSchema: ( {"chpasswd": {"list": []}}, pytest.raises( - SchemaValidationError, match=r"\[\] is too short" + SchemaValidationError, + match=rf"\[\] {SCHEMA_EMPTY_ERROR}", ), ), ], diff --git a/tests/unittests/config/test_cc_snap.py b/tests/unittests/config/test_cc_snap.py index 65088dd..573ade9 100644 --- a/tests/unittests/config/test_cc_snap.py +++ b/tests/unittests/config/test_cc_snap.py @@ -14,7 +14,12 @@ from cloudinit.config.schema import ( get_schema, validate_cloudconfig_schema, ) -from tests.unittests.helpers import CiTestCase, mock, skipUnlessJsonSchema +from tests.unittests.helpers import ( + SCHEMA_EMPTY_ERROR, + CiTestCase, + mock, + skipUnlessJsonSchema, +) from tests.unittests.util import get_cloud M_PATH = "cloudinit.config.cc_snap." @@ -288,15 +293,18 @@ class TestSnapSchema: {"snap": {"commands": ["ls"], "invalid-key": ""}}, "Additional properties are not allowed", ), - ({"snap": {}}, "{} does not have enough properties"), + ({"snap": {}}, f"{{}} {SCHEMA_EMPTY_ERROR}"), ( {"snap": {"commands": "broken"}}, "'broken' is not of type 'object', 'array'", ), - ({"snap": {"commands": []}}, r"snap.commands: \[\] is too short"), + ( + {"snap": {"commands": []}}, + rf"snap.commands: \[\] {SCHEMA_EMPTY_ERROR}", + ), ( {"snap": {"commands": {}}}, - r"snap.commands: {} does not have enough properties", + rf"snap.commands: {{}} {SCHEMA_EMPTY_ERROR}", ), ({"snap": {"commands": [123]}}, ""), ({"snap": {"commands": {"01": 123}}}, ""), @@ -311,10 +319,10 @@ class TestSnapSchema: {"snap": {"assertions": "broken"}}, "'broken' is not of type 'object', 'array'", ), - ({"snap": {"assertions": []}}, r"\[\] is too short"), + ({"snap": {"assertions": []}}, rf"\[\] {SCHEMA_EMPTY_ERROR}"), ( {"snap": {"assertions": {}}}, - r"\{} does not have enough properties", + rf"\{{}} {SCHEMA_EMPTY_ERROR}", ), ], ) diff --git a/tests/unittests/config/test_cc_write_files.py b/tests/unittests/config/test_cc_write_files.py index 210edf7..ea9cf8a 100644 --- a/tests/unittests/config/test_cc_write_files.py +++ b/tests/unittests/config/test_cc_write_files.py @@ -18,6 +18,7 @@ from cloudinit.config.schema import ( validate_cloudconfig_schema, ) from tests.unittests.helpers import ( + SCHEMA_EMPTY_ERROR, CiTestCase, FilesystemMockingTestCase, skipUnlessJsonSchema, @@ -222,7 +223,10 @@ class TestWriteFilesSchema: [ # Top-level write_files type validation ({"write_files": 1}, "write_files: 1 is not of type 'array'"), - ({"write_files": []}, re.escape("write_files: [] is too short")), + ( + {"write_files": []}, + re.escape("write_files: [] ") + SCHEMA_EMPTY_ERROR, + ), ( {"write_files": [{}]}, "write_files.0: 'path' is a required property", diff --git a/tests/unittests/config/test_cc_yum_add_repo.py b/tests/unittests/config/test_cc_yum_add_repo.py index d2c2912..e7392f2 100644 --- a/tests/unittests/config/test_cc_yum_add_repo.py +++ b/tests/unittests/config/test_cc_yum_add_repo.py @@ -139,7 +139,7 @@ class TestAddYumRepoSchema: ), ( {"yum_repos": {}}, - re.escape("yum_repos: {} does not have enough properties"), + re.escape("yum_repos: {} ") + helpers.SCHEMA_EMPTY_ERROR, ), # baseurl required ( diff --git a/tests/unittests/helpers.py b/tests/unittests/helpers.py index 96f407f..42cc2e9 100644 --- a/tests/unittests/helpers.py +++ b/tests/unittests/helpers.py @@ -47,6 +47,14 @@ except ImportError: HAS_APT_PKG = False +# Used by tests to verify the error message when a jsonschema structure +# is empty but should not be. +# Version 4.20.0 of jsonschema changed the error messages for empty structures. +SCHEMA_EMPTY_ERROR = ( + "(is too short|should be non-empty|does not have enough properties)" +) + + # Makes the old path start # with new base instead of whatever # it previously had diff --git a/tests/unittests/test_merging.py b/tests/unittests/test_merging.py index 891031e..52abaa7 100644 --- a/tests/unittests/test_merging.py +++ b/tests/unittests/test_merging.py @@ -270,7 +270,7 @@ class TestMergingSchema: [ ({"merge_how": "list()+dict()+str()"}, None), ({"merge_type": "list()+dict()+str()"}, None), - ({"merge_how": []}, "\\[\\] is too short"), + ({"merge_how": []}, f"\\[\\] {helpers.SCHEMA_EMPTY_ERROR}"), ( {"merge_how": {"name": "list", "settings": ["append"]}}, "is not of type", -- 2.27.0