patchtest: improve test issue messages

The patchtest tests provide vague feedback to the user, and many of them
also provide redundant 'fix' strings that could easily be incorporated
into the issue messages themselves. Simplify them so that it is more
clear what the errors are and how they can be addressed. No
recommendation is given when the issue string adequately conveys the
issue, e.g. with a missing "LICENSE" entry in a newly-created recipe.

Signed-off-by: Trevor Gamblin <tgamblin@baylibre.com>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>

diff --git a/meta/lib/patchtest/tests/test_mbox_author.py b/meta/lib/patchtest/tests/test_mbox_author.py
index 6c79f164d48e4a157f7378ccc7e628c717611c4c..fb8f10e1fd6debd645de9ac834e2549bf20f3203 100644 (file)
--- a/meta/lib/patchtest/tests/test_mbox_author.py
+++ b/meta/lib/patchtest/tests/test_mbox_author.py
@@ -21,9 +21,9 @@ class Author(base.Base):
         for commit in self.commits:
             for invalid in self.invalids:
                 if invalid.search(commit.author):
-                    self.fail('Invalid author %s' % commit.author, 'Resend the series with a valid patch\'s author', commit)
+                    self.fail('Invalid author %s. Resend the series with a valid patch author' % commit.author, commit=commit)
 
     def test_non_auh_upgrade(self):
         for commit in self.commits:
             if self.auh_email in commit.payload:
-                self.fail('Invalid author %s in commit message' % self.auh_email, 'Resend the series with a valid patch\'s author', commit)
+                self.fail('Invalid author %s. Resend the series with a valid patch author' % self.auh_email, commit=commit)

diff --git a/meta/lib/patchtest/tests/test_mbox_bugzilla.py b/meta/lib/patchtest/tests/test_mbox_bugzilla.py
index e8de48bb8dae05d39edee9ad2263684cc7f6c535..aa53b77f87c3c3a83bb9821f766657048cf65f58 100644 (file)
--- a/meta/lib/patchtest/tests/test_mbox_bugzilla.py
+++ b/meta/lib/patchtest/tests/test_mbox_bugzilla.py
@@ -16,7 +16,5 @@ class Bugzilla(base.Base):
             for line in commit.commit_message.splitlines():
                 if self.rexp_detect.match(line):
                     if not self.rexp_validation.match(line):
-                        self.fail('Yocto Project bugzilla tag is not correctly formatted',
-                                  'Specify bugzilla ID in commit description with format: "[YOCTO #<bugzilla ID>]"',
-                                  commit)
+                        self.fail('Bugzilla issue ID is not correctly formatted - specify it with format: "[YOCTO #<bugzilla ID>]"', commit=commit)
 

diff --git a/meta/lib/patchtest/tests/test_mbox_cve.py b/meta/lib/patchtest/tests/test_mbox_cve.py
index f99194c094ca8433166be99406e761197de7823b..36548aa10c4b25a40969784788c6761fa0bd0f3a 100644 (file)
--- a/meta/lib/patchtest/tests/test_mbox_cve.py
+++ b/meta/lib/patchtest/tests/test_mbox_cve.py
@@ -44,6 +44,5 @@ class CVE(base.Base):
             if self.revert_shortlog_regex.match(commit.shortlog):
                 continue
             if not self.prog.search_string(commit.payload):
-                self.fail('Missing or incorrectly formatted CVE tag in mbox',
-                          'Correct or include the CVE tag in the mbox with format: "CVE: CVE-YYYY-XXXX"',
-                          commit)
+                self.fail('Missing or incorrectly formatted CVE tag in mbox. Correct or include the CVE tag in the mbox with format: "CVE: CVE-YYYY-XXXX"',
+                          commit=commit)

diff --git a/meta/lib/patchtest/tests/test_mbox_description.py b/meta/lib/patchtest/tests/test_mbox_description.py
index 7addc6b5f77c92161b2581a9cdf4b63e49fd0c12..46bedd46ce57201a5014f69d217c812c52c16de6 100644 (file)
--- a/meta/lib/patchtest/tests/test_mbox_description.py
+++ b/meta/lib/patchtest/tests/test_mbox_description.py
@@ -11,7 +11,5 @@ class CommitMessage(base.Base):
     def test_commit_message_presence(self):
         for commit in CommitMessage.commits:
             if not commit.commit_message.strip():
-                self.fail('Patch is missing a descriptive commit message',
-                          'Please include a commit message on your patch explaining the change (most importantly why the change is being made)',
-                          commit)
+                self.fail('Mbox is missing a descriptive commit message. Please include a commit message on your patch explaining the change', commit=commit)
 

diff --git a/meta/lib/patchtest/tests/test_mbox_format.py b/meta/lib/patchtest/tests/test_mbox_format.py
index 85c452ca0d920097770f391bae83d573ac52b5a6..42a8491a0982166e429cced00cb74cd5b30cfe21 100644 (file)
--- a/meta/lib/patchtest/tests/test_mbox_format.py
+++ b/meta/lib/patchtest/tests/test_mbox_format.py
@@ -11,6 +11,5 @@ class MboxFormat(base.Base):
 
     def test_mbox_format(self):
         if self.unidiff_parse_error:
-            self.fail('Series cannot be parsed correctly due to malformed diff lines',
-                      'Create the series again using git-format-patch and ensure it can be applied using git am',
+            self.fail('Series cannot be parsed correctly due to malformed diff lines. Create the series again using git-format-patch and ensure it can be applied using git am',
                       data=[('Diff line', re.sub('^.+:\s(?<!$)','',self.unidiff_parse_error))])

diff --git a/meta/lib/patchtest/tests/test_mbox_mailinglist.py b/meta/lib/patchtest/tests/test_mbox_mailinglist.py
index de38e205b1ecbf4c00fad7d8cdefbd6daf6b40f7..1f9e0be07f8977df5b733994423aba69348a2367 100644 (file)
--- a/meta/lib/patchtest/tests/test_mbox_mailinglist.py
+++ b/meta/lib/patchtest/tests/test_mbox_mailinglist.py
@@ -43,16 +43,15 @@ class MailingList(base.Base):
         for commit in MailingList.commits:
             match = project_regex.match(commit.subject)
             if match:
-                self.fail('Series sent to the wrong mailing list',
-                          'Check the project\'s README (%s) and send the patch to the indicated list' % match.group('project'),
-                          commit)
+                self.fail('Series sent to the wrong mailing list. Check the project\'s README (%s) and send the patch to the indicated list' % match.group('project'),
+                          commit=commit)
 
         for patch in self.patchset:
             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:
-                    self.fail('Series sent to the wrong mailing list or some patches from the series correspond to different mailing lists', 'Send the series again to the correct mailing list (ML)',
+                    self.fail('Series sent to the wrong mailing list or some patches from the series correspond to different mailing lists. Send the series again to the correct mailing list (ML)',
                               data=[('Suggested ML', '%s [%s]' % (project.listemail, project.gitrepo)),
                                     ('Patch\'s path:', patch.path)])
 
@@ -60,5 +59,5 @@ class MailingList(base.Base):
             if base_path.startswith('scripts'):
                 for poky_file in self.poky_scripts:
                     if patch.path.startswith(poky_file):
-                        self.fail('Series sent to the wrong mailing list or some patches from the series correspond to different mailing lists', 'Send the series again to the correct mailing list (ML)',
+                        self.fail('Series sent to the wrong mailing list or some patches from the series correspond to different mailing lists. Send the series again to the correct mailing list (ML)',
                                   data=[('Suggested ML', '%s [%s]' % (self.poky.listemail, self.poky.gitrepo)),('Patch\'s path:', patch.path)])

diff --git a/meta/lib/patchtest/tests/test_mbox_merge.py b/meta/lib/patchtest/tests/test_mbox_merge.py
index c8b6718d152c349c71beac281c1b8166e675e811..a7e0821e726dc1f42b76e69283226663ae00d88b 100644 (file)
--- a/meta/lib/patchtest/tests/test_mbox_merge.py
+++ b/meta/lib/patchtest/tests/test_mbox_merge.py
@@ -20,6 +20,5 @@ class Merge(base.Base):
     def test_series_merge_on_head(self):
         if not PatchTestInput.repo.ismerged:
             commithash, author, date, shortlog = headlog()
-            self.fail('Series does not apply on top of target branch',
-                      'Rebase your series on top of targeted branch',
+            self.fail('Series does not apply on top of target branch. Rebase your series and ensure the target is correct',
                       data=[('Targeted branch', '%s (currently at %s)' % (PatchTestInput.repo.branch, commithash))])

diff --git a/meta/lib/patchtest/tests/test_mbox_shortlog.py b/meta/lib/patchtest/tests/test_mbox_shortlog.py
index b6c2a209ff617aa2cecbb63b087d80bd158f900a..7cc71562f6bb1a250736117e58b91d5642fe7ec8 100644 (file)
--- a/meta/lib/patchtest/tests/test_mbox_shortlog.py
+++ b/meta/lib/patchtest/tests/test_mbox_shortlog.py
@@ -24,9 +24,8 @@ class Shortlog(base.Base):
                 try:
                     parse_shortlog.shortlog.parseString(shortlog)
                 except pyparsing.ParseException as pe:
-                    self.fail('Shortlog does not follow expected format',
-                              'Commit shortlog (first line of commit message) should follow the format "<target>: <summary>"',
-                              commit)
+                    self.fail('Commit shortlog (first line of commit message) should follow the format "<target>: <summary>"',
+                              commit=commit)
 
     def test_shortlog_length(self):
         for commit in Shortlog.commits:
@@ -36,6 +35,5 @@ class Shortlog(base.Base):
                 continue
             l = len(shortlog)
             if l > maxlength:
-                self.fail('Commit shortlog is too long',
-                          'Edit shortlog so that it is %d characters or less (currently %d characters)' % (maxlength, l),
-                          commit)
+                self.fail('Edit shortlog so that it is %d characters or less (currently %d characters)' % (maxlength, l),
+                          commit=commit)

diff --git a/meta/lib/patchtest/tests/test_mbox_signed_off_by.py b/meta/lib/patchtest/tests/test_mbox_signed_off_by.py
index 6458951f1c83b0ac1919bea8271be506e8336bf3..8fd705a2efe7c380573e09a7135ce087cb4efe8e 100644 (file)
--- a/meta/lib/patchtest/tests/test_mbox_signed_off_by.py
+++ b/meta/lib/patchtest/tests/test_mbox_signed_off_by.py
@@ -23,6 +23,5 @@ class SignedOffBy(base.Base):
             if self.revert_shortlog_regex.match(commit.shortlog):
                 continue
             if not SignedOffBy.prog.search_string(commit.payload):
-                self.fail('Patch is missing Signed-off-by',
-                          'Sign off the patch (either manually or with "git commit --amend -s")',
-                          commit)
+                self.fail('Mbox is missing Signed-off-by. Add it manually or with "git commit --amend -s"',
+                          commit=commit)

diff --git a/meta/lib/patchtest/tests/test_metadata_lic_files_chksum.py b/meta/lib/patchtest/tests/test_metadata_lic_files_chksum.py
index e9a5b6bb4ebac007c4df252092b5e9913844ad99..a25a65c6db22e5d151df10f10d13e803037c819b 100644 (file)
--- a/meta/lib/patchtest/tests/test_metadata_lic_files_chksum.py
+++ b/meta/lib/patchtest/tests/test_metadata_lic_files_chksum.py
@@ -34,8 +34,7 @@ class LicFilesChkSum(base.Metadata):
             if rd.getVar(self.license) == self.closed:
                 continue
             if not lic_files_chksum:
-                self.fail('%s is missing in newly added recipe' % self.metadata,
-                          'Specify the variable %s in %s' % (self.metadata, pn))
+                self.fail('%s is missing in newly added recipe' % self.metadata)
 
     def pretest_lic_files_chksum_modified_not_mentioned(self):
         if not self.modified:
@@ -77,6 +76,5 @@ class LicFilesChkSum(base.Metadata):
                     if self.lictag_re.search(commit.commit_message):
                        break
                 else:
-                    self.fail('LIC_FILES_CHKSUM changed on target %s but there is no "%s" tag in commit message' % (pn, self.lictag),
-                              'Include "%s: <description>" into the commit message with a brief description' % self.lictag,
+                    self.fail('LIC_FILES_CHKSUM changed on target %s but there is no "%s" tag in commit message. Include it with a brief description' % (pn, self.lictag),
                               data=[('Current checksum', pretest), ('New checksum', test)])

diff --git a/meta/lib/patchtest/tests/test_metadata_license.py b/meta/lib/patchtest/tests/test_metadata_license.py
index 16604dbfb1adafac98b29da124f69c5f18b76fc1..e49331603ced8ee86cd415fa719a1dfcbd80fc31 100644 (file)
--- a/meta/lib/patchtest/tests/test_metadata_license.py
+++ b/meta/lib/patchtest/tests/test_metadata_license.py
@@ -51,5 +51,5 @@ class License(base.Metadata):
                 fd.write(''.join(lines[:-1]))
 
         if no_license:
-            self.fail('Recipe does not have the LICENSE field set', 'Include a LICENSE into the new recipe')
+            self.fail('Recipe does not have the LICENSE field set.')
 

diff --git a/meta/lib/patchtest/tests/test_metadata_max_length.py b/meta/lib/patchtest/tests/test_metadata_max_length.py
index 04a5e23469436d540cfd053746e724b1fc715484..b3a5dc9b791b042791bde20441c0878ae2502a18 100644 (file)
--- a/meta/lib/patchtest/tests/test_metadata_max_length.py
+++ b/meta/lib/patchtest/tests/test_metadata_max_length.py
@@ -21,6 +21,5 @@ class MaxLength(base.Base):
                 if self.add_mark.match(line):
                     current_line_length = len(line[1:])
                     if current_line_length > self.max_length:
-                        self.fail('Patch line too long (current length %s)' % current_line_length,
-                                  'Shorten the corresponding patch line (max length supported %s)' % 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])])

diff --git a/meta/lib/patchtest/tests/test_metadata_src_uri.py b/meta/lib/patchtest/tests/test_metadata_src_uri.py
index 718229d7ade8a061a1761c54f0039722d2b66a9e..ce2ace17bb949ebf8bd88b7137578a6b43907d32 100644 (file)
--- a/meta/lib/patchtest/tests/test_metadata_src_uri.py
+++ b/meta/lib/patchtest/tests/test_metadata_src_uri.py
@@ -69,7 +69,6 @@ class SrcUri(base.Metadata):
                 # 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',
-                              'Amend the patch containing the software patch file removal',
+                    self.fail('Patches not removed from tree. Remove them and amend the submitted mbox',
                               data=[('Patch', f) for f in not_removed])
 

diff --git a/meta/lib/patchtest/tests/test_metadata_summary.py b/meta/lib/patchtest/tests/test_metadata_summary.py
index 931b26768e43a5daa2b9f0e1de36f56f9b324915..8bcea453c2552443179d2a89b2e93cae78dc9382 100644 (file)
--- a/meta/lib/patchtest/tests/test_metadata_summary.py
+++ b/meta/lib/patchtest/tests/test_metadata_summary.py
@@ -28,5 +28,4 @@ class Summary(base.Metadata):
 
             # "${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,
-                          'Specify the variable %s in %s' % (self.metadata, pn))
+                self.fail('%s is missing in newly added recipe' % self.metadata)

diff --git a/meta/lib/patchtest/tests/test_patch_cve.py b/meta/lib/patchtest/tests/test_patch_cve.py
index 46ed9ef791254fc195bc4731fb60237eb369b86d..144e130707d302a1b667fd0cebd55810aa49086d 100644 (file)
--- a/meta/lib/patchtest/tests/test_patch_cve.py
+++ b/meta/lib/patchtest/tests/test_patch_cve.py
@@ -46,6 +46,5 @@ class CVE(base.Base):
                         tag_found = True
                         break
                 if not tag_found:
-                    self.fail('Missing or incorrectly formatted CVE tag in included patch file',
-                              'Correct or include the CVE tag on cve patch with format: "CVE: CVE-YYYY-XXXX"',
-                              commit)
+                    self.fail('Missing or incorrectly formatted CVE tag in patch file. Correct or include the CVE tag in the patch with format: "CVE: CVE-YYYY-XXXX"',
+                              commit=commit)

diff --git a/meta/lib/patchtest/tests/test_patch_signed_off_by.py b/meta/lib/patchtest/tests/test_patch_signed_off_by.py
index 4855d6daf76703e046ea8e859ee19c49c4e4908e..5892033af084b0ff25dcbb6351e4c9d41d45ad84 100644 (file)
--- a/meta/lib/patchtest/tests/test_patch_signed_off_by.py
+++ b/meta/lib/patchtest/tests/test_patch_signed_off_by.py
@@ -39,5 +39,4 @@ class PatchSignedOffBy(base.Base):
                 if PatchSignedOffBy.prog.search_string(payload):
                     break
             else:
-                self.fail('A patch file has been added, but does not have a Signed-off-by tag',
-                          'Sign off the added patch file (%s)' % newpatch.path)
+                self.fail('A patch file has been added, but does not have a Signed-off-by tag. Sign off the added patch file (%s)' % newpatch.path)

diff --git a/meta/lib/patchtest/tests/test_patch_upstream_status.py b/meta/lib/patchtest/tests/test_patch_upstream_status.py
index eda5353c669e02cf9eaae32336398d27b71dc3ee..c21aeaf28fd6acd3f4c6a011584fc7afad6226df 100644 (file)
--- a/meta/lib/patchtest/tests/test_patch_upstream_status.py
+++ b/meta/lib/patchtest/tests/test_patch_upstream_status.py
@@ -34,8 +34,7 @@ class PatchUpstreamStatus(base.Base):
         for newpatch in PatchUpstreamStatus.newpatches:
             payload = newpatch.__str__()
             if not self.upstream_status_regex.search_string(payload):
-                self.fail('Added patch file is missing Upstream-Status in the header',
-                          'Add Upstream-Status: <Valid status> to the header of %s' % newpatch.path,
+                self.fail('Added patch file is missing Upstream-Status in the header. Add Upstream-Status: <Valid status> to the header',
                           data=[('Standard format', self.standard_format), ('Valid status', self.valid_status)])
             for line in payload.splitlines():
                 if self.patchmetadata_regex.match(line):
@@ -46,19 +45,16 @@ class PatchUpstreamStatus(base.Base):
                                 parse_upstream_status.upstream_status_inappropriate_info.parseString(line.lstrip('+'))
                             except pyparsing.ParseException as pe:
                                 self.fail('Upstream-Status is Inappropriate, but no reason was provided',
-                                          'Include a brief reason why %s is inappropriate' % os.path.basename(newpatch.path),
                                           data=[('Current', pe.pstr), ('Standard format', 'Upstream-Status: Inappropriate [reason]')])
                         elif parse_upstream_status.submitted_status_mark.searchString(line):
                             try:
                                 parse_upstream_status.upstream_status_submitted_info.parseString(line.lstrip('+'))
                             except pyparsing.ParseException as pe:
                                 self.fail('Upstream-Status is Submitted, but it is not mentioned where',
-                                          'Include where %s was submitted' % os.path.basename(newpatch.path),
                                           data=[('Current', pe.pstr), ('Standard format', 'Upstream-Status: Submitted [where]')])
                         else:
                             try:
                                 parse_upstream_status.upstream_status.parseString(line.lstrip('+'))
                             except pyparsing.ParseException as pe:
                                 self.fail('Upstream-Status is in incorrect format',
-                                          'Fix Upstream-Status format in %s' % os.path.basename(newpatch.path),
                                           data=[('Current', pe.pstr), ('Standard format', self.standard_format), ('Valid status', self.valid_status)])

diff --git a/meta/lib/patchtest/tests/test_python_pylint.py b/meta/lib/patchtest/tests/test_python_pylint.py
index ea8efb7c2ac06f336f2db7a476823397aed04f7e..907bd9eef4ae4142f2cacf000d0c7cbde68d5400 100644 (file)
--- a/meta/lib/patchtest/tests/test_python_pylint.py
+++ b/meta/lib/patchtest/tests/test_python_pylint.py
@@ -56,6 +56,5 @@ class PyLint(base.Base):
 
         for issue in self.pylint_test:
              if self.pylint_test[issue] not in self.pylint_pretest.values():
-                 self.fail('Errors in your Python code were encountered',
-                           'Correct the lines introduced by your patch',
+                 self.fail('Errors in your Python code were encountered. Please check your code with a linter and resubmit',
                            data=[('Output', 'Please, fix the listed issues:'), ('', issue + ' ' + self.pylint_test[issue])])