Do a sweep of the test suites to remove redundant code, make variable names more explicit, and fix some formatting. Some logic (such as the bits for checking valid Upstream-Status tags) are also moved directly to the relevant functions, instead of being left in the setup stage. This should make maintenance easier going forward.
AI-Generated: Uses Claude Code Signed-off-by: Trevor Gamblin <[email protected]> --- meta/lib/patchtest/tests/base.py | 4 --- meta/lib/patchtest/tests/test_mbox.py | 10 +++--- meta/lib/patchtest/tests/test_metadata.py | 32 ++++++------------ meta/lib/patchtest/tests/test_patch.py | 33 ++++++++----------- .../lib/patchtest/tests/test_python_pylint.py | 8 ++--- 5 files changed, 33 insertions(+), 54 deletions(-) diff --git a/meta/lib/patchtest/tests/base.py b/meta/lib/patchtest/tests/base.py index 919ca136bb..8263932dda 100644 --- a/meta/lib/patchtest/tests/base.py +++ b/meta/lib/patchtest/tests/base.py @@ -22,10 +22,6 @@ info = logger.info warn = logger.warn error = logger.error -Commit = collections.namedtuple( - "Commit", ["author", "subject", "commit_message", "shortlog", "payload"] -) - Commit = collections.namedtuple('Commit', ['author', 'subject', 'commit_message', 'shortlog', 'payload']) class PatchtestOEError(Exception): diff --git a/meta/lib/patchtest/tests/test_mbox.py b/meta/lib/patchtest/tests/test_mbox.py index 55d1068df3..f158f4472c 100644 --- a/meta/lib/patchtest/tests/test_mbox.py +++ b/meta/lib/patchtest/tests/test_mbox.py @@ -65,11 +65,11 @@ class TestMbox(base.Base): shortlog = re.sub(r'^(\[.*?\])+ ', '', commit.shortlog) if shortlog.startswith('Revert "'): continue - l = len(shortlog) - if l > patchtest_patterns.mbox_shortlog_maxlength: + length = len(shortlog) + if length > patchtest_patterns.mbox_shortlog_maxlength: self.fail( "Edit shortlog so that it is %d characters or less (currently %d characters)" - % (patchtest_patterns.mbox_shortlog_maxlength, l), + % (patchtest_patterns.mbox_shortlog_maxlength, length), commit=commit, ) @@ -108,7 +108,7 @@ class TestMbox(base.Base): folders = patch.path.split('/') base_path = folders[0] for project in [self.bitbake, self.doc, self.oe, self.poky]: - if base_path in project.paths: + if base_path in project.paths: self.fail('Series sent to the wrong mailing list or some patches from the series correspond to different mailing lists', data=[('Suggested ML', '%s [%s]' % (project.listemail, project.gitrepo)), ('Patch\'s path:', patch.path)]) @@ -161,7 +161,7 @@ class TestMbox(base.Base): for commit in self.commits: if patchtest_patterns.auh_email in commit.commit_message: self.fail( - "Invalid author %s. Resend the series with a valid patch author" + "Commit message contains AUH email address %s. Resend the series with a valid patch author" % patchtest_patterns.auh_email, commit=commit, ) diff --git a/meta/lib/patchtest/tests/test_metadata.py b/meta/lib/patchtest/tests/test_metadata.py index 30da8dbe60..442e80973d 100644 --- a/meta/lib/patchtest/tests/test_metadata.py +++ b/meta/lib/patchtest/tests/test_metadata.py @@ -8,7 +8,6 @@ import base import collections import os import patchtest_patterns -import pyparsing from patchtest_parser import PatchtestParser # Data store commonly used to share values between pre and post-merge tests @@ -108,39 +107,28 @@ class TestMetadata(base.Metadata): ], ) - def pretest_src_uri_left_files(self): - # these tests just make sense on patches that can be merged - if not PatchtestParser.repo.canbemerged(): - self.skip("Patch cannot be merged") - if not self.modified: - self.skip('No modified recipes, skipping pretest') - - # get the proper metadata values + def _collect_src_uri(self, key_prefix): for pn in self.modified: - # we are not interested in images if 'core-image' in pn: continue rd = self.tinfoil.parse_recipe(pn) PatchTestDataStore[ - "%s-%s-%s" % (self.shortid(), patchtest_patterns.metadata_src_uri, pn) + "%s-%s-%s" % (key_prefix, patchtest_patterns.metadata_src_uri, pn) ] = rd.getVar(patchtest_patterns.metadata_src_uri) - def test_src_uri_left_files(self): - # these tests just make sense on patches that can be merged + def pretest_src_uri_left_files(self): if not PatchtestParser.repo.canbemerged(): self.skip("Patch cannot be merged") if not self.modified: self.skip('No modified recipes, skipping pretest') + self._collect_src_uri(self.shortid()) - # get the proper metadata values - for pn in self.modified: - # we are not interested in images - if 'core-image' in pn: - continue - rd = self.tinfoil.parse_recipe(pn) - PatchTestDataStore[ - "%s-%s-%s" % (self.shortid(), patchtest_patterns.metadata_src_uri, pn) - ] = rd.getVar(patchtest_patterns.metadata_src_uri) + def test_src_uri_left_files(self): + if not PatchtestParser.repo.canbemerged(): + self.skip("Patch cannot be merged") + if not self.modified: + self.skip('No modified recipes, skipping test') + self._collect_src_uri(self.shortid()) for pn in self.modified: pretest_src_uri = PatchTestDataStore[ diff --git a/meta/lib/patchtest/tests/test_patch.py b/meta/lib/patchtest/tests/test_patch.py index 3b33b5a199..9f1d7d2c8f 100644 --- a/meta/lib/patchtest/tests/test_patch.py +++ b/meta/lib/patchtest/tests/test_patch.py @@ -28,21 +28,16 @@ class TestPatch(base.Base): def setUp(self): if self.unidiff_parse_error: self.skip('Parse error %s' % self.unidiff_parse_error) - - self.valid_status = ", ".join(patchtest_patterns.upstream_status_nonliteral_valid_status) - self.standard_format = "Upstream-Status: <Valid status>" - - # we are just interested in series that introduce CVE patches, thus discard other - # possibilities: modification to current CVEs, patch directly introduced into the - # recipe, upgrades already including the CVE, etc. - new_cves = [p for p in self.patchset if p.path.endswith('.patch') and p.is_added_file] - if not new_cves: + if not TestPatch.newpatches: self.skip('No new CVE patches introduced') def test_upstream_status_presence_format(self): if not TestPatch.newpatches: self.skip("There are no new software patches, no reason to test Upstream-Status presence/format") + valid_status = ", ".join(patchtest_patterns.upstream_status_nonliteral_valid_status) + standard_format = "Upstream-Status: <Valid status>" + for newpatch in TestPatch.newpatches: payload = newpatch.__str__() lines = payload.splitlines() @@ -71,16 +66,16 @@ class TestPatch(base.Base): 'Upstream-Status is present only after the patch scissors. ' "It must be placed in the patch header before the scissors line.", data=[ - ("Standard format", self.standard_format), - ("Valid status", self.valid_status), + ("Standard format", standard_format), + ("Valid status", valid_status), ], ) else: self.fail( "Added patch file is missing Upstream-Status: <Valid status> in the commit message", data=[ - ("Standard format", self.standard_format), - ("Valid status", self.valid_status), + ("Standard format", standard_format), + ("Valid status", valid_status), ], ) @@ -97,8 +92,8 @@ class TestPatch(base.Base): "but was found afterwards.", data=[ ("Current", line.lstrip("+")), - ("Standard format", self.standard_format), - ("Valid status", self.valid_status), + ("Standard format", standard_format), + ("Valid status", valid_status), ], ) @@ -142,14 +137,14 @@ class TestPatch(base.Base): "Upstream-Status is in incorrect format", data=[ ("Current", pe.pstr), - ("Standard format", self.standard_format), - ("Valid status", self.valid_status), + ("Standard format", standard_format), + ("Valid status", valid_status), ], ) def test_signed_off_by_presence(self): if not TestPatch.newpatches: - self.skip("There are no new software patches, no reason to test %s presence" % PatchSignedOffBy.mark) + self.skip("There are no new software patches, no reason to test %s presence" % TestPatch.mark) for newpatch in TestPatch.newpatches: payload = newpatch.__str__() @@ -162,7 +157,7 @@ class TestPatch(base.Base): self.fail('A patch file has been added without a Signed-off-by tag: \'%s\'' % os.path.basename(newpatch.path)) def test_cve_tag_format(self): - for commit in TestPatch.commits: + for commit in self.commits: if patchtest_patterns.cve.search_string( commit.shortlog ) or patchtest_patterns.cve.search_string(commit.commit_message): diff --git a/meta/lib/patchtest/tests/test_python_pylint.py b/meta/lib/patchtest/tests/test_python_pylint.py index ec9129bc79..37d1de9436 100644 --- a/meta/lib/patchtest/tests/test_python_pylint.py +++ b/meta/lib/patchtest/tests/test_python_pylint.py @@ -47,7 +47,7 @@ class PyLint(base.Base): for pythonpatch in self.pythonpatches: # a condition checking whether a file is renamed or not # unidiff doesn't support this yet - if pythonpatch.target_file is not pythonpatch.path: + if pythonpatch.target_file != pythonpatch.path: path = pythonpatch.target_file[2:] else: path = pythonpatch.path @@ -55,9 +55,9 @@ class PyLint(base.Base): reporter = TextReporter(pylint_output) lint.Run([self.pylint_options, pythonpatch.path], reporter=reporter, exit=False) for line in pylint_output.readlines(): - if not '*' in line: - if line.strip(): - self.pylint_test[line.strip().split(' ',1)[0]] = line.strip().split(' ',1)[1] + if not '*' in line: + if line.strip(): + self.pylint_test[line.strip().split(' ',1)[0]] = line.strip().split(' ',1)[1] for issue in self.pylint_test: if self.pylint_test[issue] not in self.pylint_pretest.values(): -- 2.54.0
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#237050): https://lists.openembedded.org/g/openembedded-core/message/237050 Mute This Topic: https://lists.openembedded.org/mt/119319578/21656 Group Owner: [email protected] Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
