From patchwork Thu Feb 15 21:39:32 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Simone_Wei=C3=9F?= X-Patchwork-Id: 39478 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from aws-us-west-2-korg-lkml-1.web.codeaurora.org (localhost.localdomain [127.0.0.1]) by smtp.lore.kernel.org (Postfix) with ESMTP id 14011C4829E for ; Thu, 15 Feb 2024 21:39:55 +0000 (UTC) Received: from mout01.posteo.de (mout01.posteo.de [185.67.36.65]) by mx.groups.io with SMTP id smtpd.web11.4016.1708033188988482586 for ; Thu, 15 Feb 2024 13:39:49 -0800 Authentication-Results: mx.groups.io; dkim=fail reason="dkim: body hash did not verify" header.i=@posteo.com header.s=2017 header.b=XCOige1b; spf=pass (domain: posteo.com, ip: 185.67.36.65, mailfrom: simone.p.weiss@posteo.com) Received: from submission (posteo.de [185.67.36.169]) by mout01.posteo.de (Postfix) with ESMTPS id 223BF240027 for ; Thu, 15 Feb 2024 22:39:46 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.com; s=2017; t=1708033187; bh=Y6QCBSx5qoX6bk64vrIt34oBo5w5FWkky4gnrrBqOkc=; h=From:To:Cc:Subject:Date:Message-Id:MIME-Version:Content-Type: Content-Transfer-Encoding:From; b=XCOige1bi6UyqbDA/nLY1P8mjcIjgHNSoYAKlR5jqVaLJ/FcnlF6ciJdlm9+UorSG x58YRlEQ3Old1hsEaqKCZ2BuDSW5kmmfSbChMs/04w40Qbyxntf+5H+nITcd/M2DNL lvGjUqQ5phmqISKo3D+F0m7cHIgOq5IX645EqYnHmryWYHte8VmrJzi+LLsiCgfnQJ 6LoZeqGxfo2VFMwvPqM/afuNVUyrs1OlbPyW9dpx0zl40WVEWFJ11AiE9+mL9kO2VJ Ec4BvVLhH/FSrW3Z6xa8w+914iVrYjBXmDuf+JF+mHfqYS+sTkpzJQVcpBLESablbd wXxnITBR+CtYA== Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4TbT421dD3z9rxB; Thu, 15 Feb 2024 22:39:45 +0100 (CET) From: simone.p.weiss@posteo.com To: openembedded-core@lists.openembedded.org Cc: =?utf-8?q?Simone_Wei=C3=9F?= Subject: [PATCH] patchtest: Add further information for failed testcases Date: Thu, 15 Feb 2024 21:39:32 +0000 Message-Id: <20240215213932.245929-1-simone.p.weiss@posteo.com> MIME-Version: 1.0 List-Id: X-Webhook-Received: from li982-79.members.linode.com [45.33.32.79] by aws-us-west-2-korg-lkml-1.web.codeaurora.org with HTTPS for ; Thu, 15 Feb 2024 21:39:55 -0000 X-Groupsio-URL: https://lists.openembedded.org/g/openembedded-core/message/195706 From: Simone Weiß 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ß --- 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 #]"', commit=commit) + self.fail('Bugzilla issue ID is not correctly formatted - specify it with format: "[YOCTO #]. 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: in the commit message', + self.fail('Added patch file is missing Upstream-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):