Created attachment 5009 [details] Patch without a Signed-off-by line I'm testing this on Poky master... Given the attached patch, patchtest reports a failure but says "OK" at the end: Testing patch v2-0002-alsa-tools-upgrade-1.2.5-1.2.11.patch Loading cache: 100% | | ETA: --:--:-- Loaded 0 entries from dependency cache. Parsing recipes: 100% |#############################################################################################################################################################################| Time: 0:00:13 Parsing of 912 .bb files complete (0 cached, 912 parsed). 1849 targets, 47 skipped, 0 masked, 0 errors. SKIP: pretest src uri left files: Patch cannot be merged (test_metadata.TestMetadata.pretest_src_uri_left_files) SKIP: pretest pylint: No python related patches, skipping test (test_python_pylint.PyLint.pretest_pylint) ---------------------------------------------------------------------- Ran 2 tests in 14.104s OK PASS: test author valid (test_mbox.TestMbox.test_author_valid) SKIP: test bugzilla entry format: No bug ID found (test_mbox.TestMbox.test_bugzilla_entry_format) PASS: test commit message presence (test_mbox.TestMbox.test_commit_message_presence) PASS: test mbox format (test_mbox.TestMbox.test_mbox_format) PASS: test non-AUH upgrade (test_mbox.TestMbox.test_non_auh_upgrade) SKIP: test series merge on head: Merge test is disabled for now (test_mbox.TestMbox.test_series_merge_on_head) PASS: test shortlog format (test_mbox.TestMbox.test_shortlog_format) PASS: test shortlog length (test_mbox.TestMbox.test_shortlog_length) FAIL: test Signed-off-by presence: Mbox is missing Signed-off-by. Add it manually or with "git commit --amend -s" (test_mbox.TestMbox.test_signed_off_by_presence) PASS: test target mailing list (test_mbox.TestMbox.test_target_mailing_list) Loading cache: 100% |###############################################################################################################################################################################| Time: 0:00:00 Loaded 1849 entries from dependency cache. PASS: test CVE check ignore (test_metadata.TestMetadata.test_cve_check_ignore) PASS: test lic files chksum modified not mentioned (test_metadata.TestMetadata.test_lic_files_chksum_modified_not_mentioned) SKIP: test lic files chksum presence: No added recipes, skipping test (test_metadata.TestMetadata.test_lic_files_chksum_presence) SKIP: test license presence: No added recipes, skipping test (test_metadata.TestMetadata.test_license_presence) PASS: test max line length (test_metadata.TestMetadata.test_max_line_length) SKIP: test src uri left files: Patch cannot be merged (test_metadata.TestMetadata.test_src_uri_left_files) SKIP: test summary presence: No added recipes, skipping test (test_metadata.TestMetadata.test_summary_presence) SKIP: test CVE tag format: No new CVE patches introduced (test_patch.TestPatch.test_cve_tag_format) SKIP: test Signed-off-by presence: No new CVE patches introduced (test_patch.TestPatch.test_signed_off_by_presence) SKIP: test Upstream-Status presence: No new CVE patches introduced (test_patch.TestPatch.test_upstream_status_presence_format) SKIP: test pylint: No python related patches, skipping test (test_python_pylint.PyLint.test_pylint) ---------------------------------------------------------------------- Ran 21 tests in 1.099s OK Because of this, the "FAIL" line got unnoticed, and I sent the patch in good faith.
That's better, now, thanks Simone! ---------------------------------------------------------------------- patchtest: At least one patchtest caused a failure or an error - please check ---------------------------------------------------------------------- However, I think this would have been harder to miss: ---------------------------------------------------------------------- FAIL: At least one patchtest caused a failure or an error - please check ---------------------------------------------------------------------- I'm reopening the bug, but I don't mind if you disagree and close it again. It's already a good improvement, but I'm unsure people reading the output too quickly won't miss the error. Thanks again
Hi Michael, While true that adding FAIL makes it even more obvious, the problem is that this would be picked up by patchtest-send-results will pick it up and put it in the list of failed testcases. I would hence keep it as is.
Oops, I understand. "ERROR" maybe if it doesn't interfere? Anyway, I was just trying to help. Don't hesitate to close the bug again.
Sure, this is absolutely possible. I can check and send a patch later for sth like WARN, or NOK.