public inbox for openembedded-core@lists.openembedded.org
 help / color / mirror / Atom feed
* [PATCH] patchtest: Add further information for failed testcases
@ 2024-02-15 21:39 simone.p.weiss
  2024-02-15 22:10 ` [OE-core] " Richard Purdie
  0 siblings, 1 reply; 6+ messages in thread
From: simone.p.weiss @ 2024-02-15 21:39 UTC (permalink / raw)
  To: openembedded-core; +Cc: Simone Weiß

From: Simone Weiß <simone.p.weiss@posteo.com>

Add more information to log messages when a test case fails.
Still keep it short and mostly reference the documentation. Reasson is that
documentation should already contain the needed information, do not duplicate
it here, so we also do not need to update here should the doc/policy change.

Signed-off-by: Simone Weiß <simone.p.weiss@posteo.com>
---
 meta/lib/patchtest/tests/test_mbox.py     | 23 ++++++++++++++---------
 meta/lib/patchtest/tests/test_metadata.py | 11 ++++++-----
 meta/lib/patchtest/tests/test_patch.py    | 10 ++++++----
 3 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/meta/lib/patchtest/tests/test_mbox.py b/meta/lib/patchtest/tests/test_mbox.py
index 0b623b7d17..32e60bf0f5 100644
--- a/meta/lib/patchtest/tests/test_mbox.py
+++ b/meta/lib/patchtest/tests/test_mbox.py
@@ -55,6 +55,8 @@ class TestMbox(base.Base):
     poky    = Project(name='Poky', listemail='poky@yoctoproject.org', gitrepo='http://git.yoctoproject.org/cgit/cgit.cgi/poky/', paths=paths['poky'])
     oe      = Project(name='oe', listemail='openembedded-devel@lists.openembedded.org', gitrepo='http://git.openembedded.org/meta-openembedded/', paths=paths['oe'])
 
+    commit_guide = 'https://docs.yoctoproject.org/dev/contributor-guide/submit-changes.html#implement-and-commit-changes'
+    contrib_guide = 'https://docs.yoctoproject.org/dev/contributor-guide/submit-changes.html#finding-a-suitable-mailing-list'
 
     def test_signed_off_by_presence(self):
         for commit in TestMbox.commits:
@@ -88,7 +90,7 @@ class TestMbox(base.Base):
                 continue
             l = len(shortlog)
             if l > self.maxlength:
-                self.fail('Edit shortlog so that it is %d characters or less (currently %d characters)' % (self.maxlength, l),
+                self.fail('Edit shortlog (first line of commit message) so that it is %d characters or less (currently %d characters)' % (self.maxlength, l),
                           commit=commit)
 
     def test_series_merge_on_head(self):
@@ -97,7 +99,7 @@ class TestMbox(base.Base):
             self.skip("Skipping merge test since patch is not intended for master branch. Target detected is %s" % PatchTestInput.repo.branch)
         if not PatchTestInput.repo.ismerged:
             commithash, author, date, shortlog = headlog()
-            self.fail('Series does not apply on top of target branch %s' % PatchTestInput.repo.branch,
+            self.fail('Series does not apply on top of target branch %s.' % PatchTestInput.repo.branch,
                       data=[('Targeted branch', '%s (currently at %s)' % (PatchTestInput.repo.branch, commithash))])
 
     def test_target_mailing_list(self):
@@ -111,7 +113,7 @@ class TestMbox(base.Base):
         for commit in TestMbox.commits:
             match = project_regex.search_string(commit.subject)
             if match:
-                self.fail('Series sent to the wrong mailing list or some patches from the series correspond to different mailing lists',
+                self.fail('Series sent to the wrong mailing list or some patches from the series correspond to different mailing lists. See also the contributor guide: %s' % contrib_guide, 
                           commit=commit)
 
         for patch in self.patchset:
@@ -119,7 +121,7 @@ class TestMbox(base.Base):
             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',
+                    self.fail('Series sent to the wrong mailing list or some patches from the series correspond to different mailing lists. See also the contributor guide: %s' % contrib_guide, 
                               data=[('Suggested ML', '%s [%s]' % (project.listemail, project.gitrepo)),
                                     ('Patch\'s path:', patch.path)])
 
@@ -127,7 +129,7 @@ class TestMbox(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',
+                        self.fail('Series sent to the wrong mailing list or some patches from the series correspond to different mailing lists. See also the contributor guide: %s' % contrib_guide, 
                                   data=[('Suggested ML', '%s [%s]' % (self.poky.listemail, self.poky.gitrepo)),('Patch\'s path:', patch.path)])
 
     def test_mbox_format(self):
@@ -138,22 +140,25 @@ class TestMbox(base.Base):
     def test_commit_message_presence(self):
         for commit in TestMbox.commits:
             if not commit.commit_message.strip():
-                self.fail('Please include a commit message on your patch explaining the change', commit=commit)
+                self.fail('Please include a commit message on your patch explaining the change See also the guidelines under ', commit=commit)
 
     def test_bugzilla_entry_format(self):
         for commit in TestMbox.commits:
             if not self.rexp_detect.search_string(commit.commit_message):
                 self.skip("No bug ID found")
             elif not self.rexp_validation.search_string(commit.commit_message):
-                self.fail('Bugzilla issue ID is not correctly formatted - specify it with format: "[YOCTO #<bugzilla ID>]"', commit=commit)
+                self.fail('Bugzilla issue ID is not correctly formatted - specify it with format: "[YOCTO #<bugzilla ID>]. For more information, see %s:'
+						   % commit_guide, commit=commit)
 
     def test_author_valid(self):
         for commit in self.commits:
             for invalid in self.invalids:
                 if invalid.search_string(commit.author):
-                    self.fail('Invalid author %s. Resend the series with a valid patch author' % commit.author, commit=commit)
+                    self.fail('Invalid author %s. Resend the series with a valid patch author. You can update the author of a commit e.g., with git commit --amend.'
+                              % 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. Resend the series with a valid patch author' % self.auh_email, commit=commit)
+                self.fail('Invalid author %s. Resend the series with a valid patch author. You can update the author of a commit e.g., with git commit --amend.'
+                           % self.auh_email, commit=commit)
diff --git a/meta/lib/patchtest/tests/test_metadata.py b/meta/lib/patchtest/tests/test_metadata.py
index 174dfc31c6..3792f9e3bc 100644
--- a/meta/lib/patchtest/tests/test_metadata.py
+++ b/meta/lib/patchtest/tests/test_metadata.py
@@ -62,7 +62,7 @@ class TestMetadata(base.Metadata):
                 fd.write(''.join(lines[:-1]))
 
         if no_license:
-            self.fail('Recipe does not have the LICENSE field set.')
+            self.fail('Recipe does not have the LICENSE field set. For more information, see https://docs.yoctoproject.org/dev/ref-manual/variables.html#term-LICENSE.')
 
     def test_lic_files_chksum_presence(self):
         if not self.added:
@@ -78,7 +78,7 @@ class TestMetadata(base.Metadata):
             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)
+                self.fail('%s is missing in newly added recipe. For more information, see https://docs.yoctoproject.org/dev/ref-manual/variables.html#term-LIC_FILES_CHKSUM.' % self.metadata_chksum)
 
     def test_lic_files_chksum_modified_not_mentioned(self):
         if not self.modified:
@@ -163,7 +163,7 @@ class TestMetadata(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. Remove them and amend the submitted mbox',
+                    self.fail('Patches that are removed from SRC_URI are not removed from tree. Remove them and amend the submitted mbox',
                               data=[('Patch', f) for f in not_removed])
 
     def test_summary_presence(self):
@@ -179,7 +179,7 @@ class TestMetadata(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_summary)
+                self.fail('%s is missing in newly added recipe. For more information, see https://docs.yoctoproject.org/dev/ref-manual/variables.html#term-SUMMARY' % self.metadata_summary)
 
     def test_cve_check_ignore(self):
         if not self.modified:
@@ -192,4 +192,5 @@ class TestMetadata(base.Metadata):
             cve_check_ignore = rd.getVar(self.cve_check_ignore_var)
 
             if cve_check_ignore is not None:
-                self.fail('%s is deprecated and should be replaced by %s' % (self.cve_check_ignore_var, self.cve_status_var))
+                self.fail('%s is deprecated and should be replaced by %s. For more information, see https://docs.yoctoproject.org/dev/ref-manual/variables.html#term-CVE_STATUS'
+						   % (self.cve_check_ignore_var, self.cve_status_var))
diff --git a/meta/lib/patchtest/tests/test_patch.py b/meta/lib/patchtest/tests/test_patch.py
index d7187a0cb1..5032e913c3 100644
--- a/meta/lib/patchtest/tests/test_patch.py
+++ b/meta/lib/patchtest/tests/test_patch.py
@@ -17,6 +17,8 @@ class TestPatch(base.Base):
     re_cve_payload_tag = pyparsing.Regex("\+CVE:(\s+CVE\-\d{4}\-\d+)+")
     upstream_status_regex = pyparsing.AtLineStart("+" + "Upstream-Status")
 
+    upstream_guide = 'https://docs.yoctoproject.org/dev/contributor-guide/recipe-style-guide.html?highlight=upstream+status#patch-upstream-status'
+  
     @classmethod
     def setUpClassLocal(cls):
         cls.newpatches = []
@@ -51,7 +53,7 @@ class TestPatch(base.Base):
         for newpatch in TestPatch.newpatches:
             payload = newpatch.__str__()
             if not self.upstream_status_regex.search_string(payload):
-                self.fail('Added patch file is missing Upstream-Status: <Valid status> in the commit message',
+                self.fail('Added patch file is missing Upstream-Status: <Valid status> in the commit message. For more information, see %s' % upstream_guide,
                           data=[('Standard format', self.standard_format), ('Valid status', self.valid_status)])
             for line in payload.splitlines():
                 if self.patchmetadata_regex.match(line):
@@ -61,19 +63,19 @@ class TestPatch(base.Base):
                             try:
                                 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',
+                                self.fail('Upstream-Status is Inappropriate, but no reason was provided. For more information, see %s' % upstream_guide,
                                           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',
+                                self.fail('Upstream-Status is Submitted, but it is not mentioned where. For more information, see %s' % upstream_guide,
                                           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',
+                                self.fail('Upstream-Status is in incorrect format. For more information, see %s' % upstream_guide,
                                           data=[('Current', pe.pstr), ('Standard format', self.standard_format), ('Valid status', self.valid_status)])
 
     def test_signed_off_by_presence(self):
-- 
2.39.2



^ permalink raw reply related	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-02-16 20:26 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-15 21:39 [PATCH] patchtest: Add further information for failed testcases simone.p.weiss
2024-02-15 22:10 ` [OE-core] " Richard Purdie
2024-02-16 16:19   ` Simone Weiß
2024-02-16 16:43     ` Trevor Gamblin
2024-02-16 16:51       ` Richard Purdie
2024-02-16 20:26       ` Simone Weiß

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox