From 207a5a21d38f4ed1b71aed0158ce067b57502cc7 Mon Sep 17 00:00:00 2001 From: Trevor Gamblin Date: Thu, 26 Oct 2023 10:07:46 -0400 Subject: patchtest: simplify test directory structure Consolidate the various mbox tests into a new TestMbox class, metadata tests into TestMetadata, and patch tests into TestPatch. Also update the selftest filenames to match the changes. The test contents are not significantly changed (other than to reference the new class names). While this doesn't improve overall readability, it does result in more obvious categorization, and more importantly reduces the number of calls to setup tinfoil in the tests, resulting in a roughly 25% reduction in runtime. Before: [tgamblin@megalith poky]$ time ./meta/lib/patchtest/selftest/selftest XPASS: PatchSignedOffBy.test_signed_off_by_presence (file: PatchSignedOffBy.test_signed_off_by_presence.pass) XFAIL: Shortlog.test_shortlog_format (file: Shortlog.test_shortlog_format.fail) XFAIL: MboxFormat.test_mbox_format (file: MboxFormat.test_mbox_format.1.fail) XPASS: Shortlog.test_shortlog_length (file: Shortlog.test_shortlog_length.pass) XFAIL: CommitMessage.test_commit_message_presence (file: CommitMessage.test_commit_message_presence.fail) XFAIL: SrcUri.test_src_uri_left_files (file: SrcUri.test_src_uri_left_files.fail) XPASS: Author.test_author_valid (file: Author.test_author_valid.1.pass) XFAIL: LicFilesChkSum.test_lic_files_chksum_modified_not_mentioned (file: LicFilesChkSum.test_lic_files_chksum_modified_not_mentioned.fail) XPASS: CVE.test_cve_tag_format (file: CVE.test_cve_tag_format.pass) XPASS: CVE.test_cve_presence_in_commit_message (file: CVE.test_cve_presence_in_commit_message.pass) XFAIL: CVE.test_cve_tag_format (file: CVE.test_cve_tag_format.fail) XFAIL: Author.test_author_valid (file: Author.test_author_valid.1.fail) XFAIL: LicFilesChkSum.test_lic_files_chksum_presence (file: LicFilesChkSum.test_lic_files_chksum_presence.fail) XSKIP: Merge.test_series_merge_on_head (file: Merge.test_series_merge_on_head.2.skip) XPASS: MboxFormat.test_mbox_format (file: MboxFormat.test_mbox_format.pass) XFAIL: SignedOffBy.test_signed_off_by_presence (file: SignedOffBy.test_signed_off_by_presence.1.fail) XPASS: Shortlog.test_shortlog_format (file: Shortlog.test_shortlog_format.pass) XFAIL: SignedOffBy.test_signed_off_by_presence (file: SignedOffBy.test_signed_off_by_presence.2.fail) XFAIL: MboxFormat.test_mbox_format (file: MboxFormat.test_mbox_format.2.fail) XFAIL: Summary.test_summary_presence (file: Summary.test_summary_presence.fail) XPASS: Author.test_author_valid (file: Author.test_author_valid.2.pass) XSKIP: Merge.test_series_merge_on_head (file: Merge.test_series_merge_on_head.1.skip) XPASS: Bugzilla.test_bugzilla_entry_format (file: Bugzilla.test_bugzilla_entry_format.pass) XFAIL: CVE.test_cve_presence_in_commit_message (file: CVE.test_cve_presence_in_commit_message.fail) XPASS: SignedOffBy.test_signed_off_by_presence (file: SignedOffBy.test_signed_off_by_presence.pass) XPASS: LicFilesChkSum.test_lic_files_chksum_presence (file: LicFilesChkSum.test_lic_files_chksum_presence.pass) XPASS: CommitMessage.test_commit_message_presence (file: CommitMessage.test_commit_message_presence.pass) XPASS: Summary.test_summary_presence (file: Summary.test_summary_presence.pass) XPASS: LicFilesChkSum.test_lic_files_chksum_modified_not_mentioned (file: LicFilesChkSum.test_lic_files_chksum_modified_not_mentioned.pass) XFAIL: Shortlog.test_shortlog_length (file: Shortlog.test_shortlog_length.fail) XFAIL: PatchSignedOffBy.test_signed_off_by_presence (file: PatchSignedOffBy.test_signed_off_by_presence.fail) XFAIL: Bugzilla.test_bugzilla_entry_format (file: Bugzilla.test_bugzilla_entry_format.fail) XPASS: SrcUri.test_src_uri_left_files (file: SrcUri.test_src_uri_left_files.pass) XFAIL: Author.test_author_valid (file: Author.test_author_valid.2.fail) ============================================================================ Testsuite summary for patchtest ============================================================================ ============================================================================ real 24m14.386s user 1m13.599s sys 0m21.477s After: [tgamblin@megalith poky]$ time ./meta/lib/patchtest/selftest/selftest XFAIL: TestMbox.test_bugzilla_entry_format (file: TestMbox.test_bugzilla_entry_format.fail) XPASS: TestMetadata.test_summary_presence (file: TestMetadata.test_summary_presence.pass) XFAIL: TestMbox.test_mbox_format (file: TestMbox.test_mbox_format.1.fail) XFAIL: TestMetadata.test_src_uri_left_files (file: TestMetadata.test_src_uri_left_files.fail) XSKIP: TestMbox.test_series_merge_on_head (file: TestMbox.test_series_merge_on_head.2.skip) XPASS: TestMbox.test_commit_message_presence (file: TestMbox.test_commit_message_presence.pass) XFAIL: TestMbox.test_commit_message_presence (file: TestMbox.test_commit_message_presence.fail) XPASS: TestMbox.test_signed_off_by_presence (file: TestMbox.test_signed_off_by_presence.pass) XFAIL: TestPatch.test_cve_tag_format (file: TestPatch.test_cve_tag_format.fail) XFAIL: TestMbox.test_author_valid (file: TestMbox.test_author_valid.1.fail) XFAIL: TestMbox.test_shortlog_length (file: TestMbox.test_shortlog_length.fail) XPASS: TestMbox.test_mbox_format (file: TestMbox.test_mbox_format.pass) XFAIL: TestPatch.test_signed_off_by_presence (file: TestPatch.test_signed_off_by_presence.fail) XFAIL: TestMbox.test_shortlog_format (file: TestMbox.test_shortlog_format.fail) XFAIL: TestMbox.test_mbox_format (file: TestMbox.test_mbox_format.2.fail) XPASS: TestPatch.test_cve_tag_format (file: TestPatch.test_cve_tag_format.pass) XSKIP: TestMbox.test_series_merge_on_head (file: TestMbox.test_series_merge_on_head.1.skip) XPASS: TestMbox.test_author_valid (file: TestMbox.test_author_valid.2.pass) XPASS: TestMetadata.test_lic_files_chksum_modified_not_mentioned (file: TestMetadata.test_lic_files_chksum_modified_not_mentioned.pass) XPASS: TestMbox.test_bugzilla_entry_format (file: TestMbox.test_bugzilla_entry_format.pass) XPASS: TestMetadata.test_src_uri_left_files (file: TestMetadata.test_src_uri_left_files.pass) XPASS: TestMetadata.test_lic_files_chksum_presence (file: TestMetadata.test_lic_files_chksum_presence.pass) XPASS: TestMbox.test_cve_presence_in_commit_message (file: TestMbox.test_cve_presence_in_commit_message.pass) XFAIL: TestMbox.test_signed_off_by_presence (file: TestMbox.test_signed_off_by_presence.2.fail) XFAIL: TestMbox.test_author_valid (file: TestMbox.test_author_valid.2.fail) XFAIL: TestMetadata.test_lic_files_chksum_presence (file: TestMetadata.test_lic_files_chksum_presence.fail) XPASS: TestMbox.test_shortlog_format (file: TestMbox.test_shortlog_format.pass) XPASS: TestMbox.test_author_valid (file: TestMbox.test_author_valid.1.pass) XPASS: TestPatch.test_signed_off_by_presence (file: TestPatch.test_signed_off_by_presence.pass) XFAIL: TestMetadata.test_lic_files_chksum_modified_not_mentioned (file: TestMetadata.test_lic_files_chksum_modified_not_mentioned.fail) XPASS: TestMbox.test_shortlog_length (file: TestMbox.test_shortlog_length.pass) XFAIL: TestMbox.test_signed_off_by_presence (file: TestMbox.test_signed_off_by_presence.1.fail) XFAIL: TestMbox.test_cve_presence_in_commit_message (file: TestMbox.test_cve_presence_in_commit_message.fail) XFAIL: TestMetadata.test_summary_presence (file: TestMetadata.test_summary_presence.fail) ============================================================================ Testsuite summary for patchtest ============================================================================ ============================================================================ real 18m39.749s user 0m41.857s sys 0m14.708s (From OE-Core rev: f788592da2fd0e21638ce2c3326675a060ba51cf) Signed-off-by: Trevor Gamblin Signed-off-by: Richard Purdie --- meta/lib/patchtest/tests/test_metadata.py | 204 ++++++++++++++++++++++++++++++ 1 file changed, 204 insertions(+) create mode 100644 meta/lib/patchtest/tests/test_metadata.py (limited to 'meta/lib/patchtest/tests/test_metadata.py') diff --git a/meta/lib/patchtest/tests/test_metadata.py b/meta/lib/patchtest/tests/test_metadata.py new file mode 100644 index 0000000000..34e119174f --- /dev/null +++ b/meta/lib/patchtest/tests/test_metadata.py @@ -0,0 +1,204 @@ +# Checks related to the patch's LIC_FILES_CHKSUM metadata variable +# +# Copyright (C) 2016 Intel Corporation +# +# SPDX-License-Identifier: GPL-2.0-only + +import base +import os +import pyparsing +from data import PatchTestInput, PatchTestDataStore + +class TestMetadata(base.Metadata): + metadata_lic = 'LICENSE' + invalid_license = 'PATCHTESTINVALID' + metadata_chksum = 'LIC_FILES_CHKSUM' + license_var = 'LICENSE' + closed = 'CLOSED' + lictag_re = pyparsing.AtLineStart("License-Update:") + add_mark = pyparsing.Regex('\+ ') + max_length = 200 + metadata_src_uri = 'SRC_URI' + md5sum = 'md5sum' + sha256sum = 'sha256sum' + git_regex = pyparsing.Regex('^git\:\/\/.*') + metadata_summary = 'SUMMARY' + + def test_license_presence(self): + if not self.added: + self.skip('No added recipes, skipping test') + + # TODO: this is a workaround so we can parse the recipe not + # containing the LICENSE var: add some default license instead + # of INVALID into auto.conf, then remove this line at the end + auto_conf = os.path.join(os.environ.get('BUILDDIR'), 'conf', 'auto.conf') + open_flag = 'w' + if os.path.exists(auto_conf): + open_flag = 'a' + with open(auto_conf, open_flag) as fd: + for pn in self.added: + fd.write('LICENSE ??= "%s"\n' % self.invalid_license) + + no_license = False + for pn in self.added: + rd = self.tinfoil.parse_recipe(pn) + license = rd.getVar(self.metadata_lic) + if license == self.invalid_license: + no_license = True + break + + # remove auto.conf line or the file itself + if open_flag == 'w': + os.remove(auto_conf) + else: + fd = open(auto_conf, 'r') + lines = fd.readlines() + fd.close() + with open(auto_conf, 'w') as fd: + fd.write(''.join(lines[:-1])) + + if no_license: + self.fail('Recipe does not have the LICENSE field set.') + + def test_lic_files_chksum_presence(self): + if not self.added: + self.skip('No added recipes, skipping test') + + for pn in self.added: + rd = self.tinfoil.parse_recipe(pn) + pathname = rd.getVar('FILE') + # we are not interested in images + if '/images/' in pathname: + continue + lic_files_chksum = rd.getVar(self.metadata_chksum) + if rd.getVar(self.license_var) == self.closed: + continue + if not lic_files_chksum: + self.fail('%s is missing in newly added recipe' % self.metadata_chksum) + + def pretest_lic_files_chksum_modified_not_mentioned(self): + if not self.modified: + self.skip('No modified recipes, skipping pretest') + # get the proper metadata values + for pn in self.modified: + rd = self.tinfoil.parse_recipe(pn) + pathname = rd.getVar('FILE') + # we are not interested in images + if '/images/' in pathname: + continue + PatchTestDataStore['%s-%s-%s' % (self.shortid(),self.metadata_chksum,pn)] = rd.getVar(self.metadata_chksum) + + def test_lic_files_chksum_modified_not_mentioned(self): + if not self.modified: + self.skip('No modified recipes, skipping test') + + # get the proper metadata values + for pn in self.modified: + rd = self.tinfoil.parse_recipe(pn) + pathname = rd.getVar('FILE') + # we are not interested in images + if '/images/' in pathname: + continue + PatchTestDataStore['%s-%s-%s' % (self.shortid(),self.metadata_chksum,pn)] = rd.getVar(self.metadata_chksum) + # compare if there were changes between pre-merge and merge + for pn in self.modified: + pretest = PatchTestDataStore['pre%s-%s-%s' % (self.shortid(),self.metadata_chksum, pn)] + test = PatchTestDataStore['%s-%s-%s' % (self.shortid(),self.metadata_chksum, pn)] + + # TODO: this is workaround to avoid false-positives when pretest metadata is empty (not reason found yet) + # For more info, check bug 12284 + if not pretest: + return + + if pretest != test: + # if any patch on the series contain reference on the metadata, fail + for commit in self.commits: + if self.lictag_re.search_string(commit.commit_message): + break + else: + self.fail('LIC_FILES_CHKSUM changed on target %s but there is no "License-Update:" tag in commit message. Include it with a brief description' % pn, + data=[('Current checksum', pretest), ('New checksum', test)]) + + def test_max_line_length(self): + for patch in self.patchset: + # for the moment, we are just interested in metadata + if patch.path.endswith('.patch'): + continue + payload = str(patch) + for line in payload.splitlines(): + if self.add_mark.search_string(line): + current_line_length = len(line[1:]) + if current_line_length > self.max_length: + self.fail('Patch line too long (current length %s, maximum is %s)' % (current_line_length, self.max_length), + data=[('Patch', patch.path), ('Line', '%s ...' % line[0:80])]) + + def pretest_src_uri_left_files(self): + # these tests just make sense on patches that can be merged + if not PatchTestInput.repo.canbemerged: + self.skip('Patch cannot be merged') + if not self.modified: + self.skip('No modified recipes, skipping pretest') + + # 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(), self.metadata_src_uri, pn)] = rd.getVar(self.metadata_src_uri) + + def test_src_uri_left_files(self): + # these tests just make sense on patches that can be merged + if not PatchTestInput.repo.canbemerged: + self.skip('Patch cannot be merged') + if not self.modified: + self.skip('No modified recipes, skipping pretest') + + # 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(), self.metadata_src_uri, pn)] = rd.getVar(self.metadata_src_uri) + + for pn in self.modified: + pretest_src_uri = PatchTestDataStore['pre%s-%s-%s' % (self.shortid(), self.metadata_src_uri, pn)].split() + test_src_uri = PatchTestDataStore['%s-%s-%s' % (self.shortid(), self.metadata_src_uri, pn)].split() + + pretest_files = set([os.path.basename(patch) for patch in pretest_src_uri if patch.startswith('file://')]) + test_files = set([os.path.basename(patch) for patch in test_src_uri if patch.startswith('file://')]) + + # check if files were removed + if len(test_files) < len(pretest_files): + + # get removals from patchset + filesremoved_from_patchset = set() + for patch in self.patchset: + if patch.is_removed_file: + filesremoved_from_patchset.add(os.path.basename(patch.path)) + + # get the deleted files from the SRC_URI + filesremoved_from_usr_uri = pretest_files - test_files + + # finally, get those patches removed at SRC_URI and not removed from the patchset + # TODO: we are not taking into account renames, so test may raise false positives + not_removed = filesremoved_from_usr_uri - filesremoved_from_patchset + if not_removed: + self.fail('Patches not removed from tree. Remove them and amend the submitted mbox', + data=[('Patch', f) for f in not_removed]) + + def test_summary_presence(self): + if not self.added: + self.skip('No added recipes, skipping test') + + for pn in self.added: + # we are not interested in images + if 'core-image' in pn: + continue + rd = self.tinfoil.parse_recipe(pn) + summary = rd.getVar(self.metadata_summary) + + # "${PN} version ${PN}-${PR}" is the default, so fail if default + if summary.startswith('%s version' % pn): + self.fail('%s is missing in newly added recipe' % self.metadata_summary) -- cgit v1.2.3-54-g00ecf