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 <tgamblin@baylibre.com>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>

1 files changed, 204 insertions, 0 deletions

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)