From 8f1fea025d78c26046d3135e026f097dc2022698 Mon Sep 17 00:00:00 2001 From: Bruce Dawson Date: Thu, 2 Jun 2022 00:26:50 +0000 Subject: [PATCH] Reland "Fix not-run-test detection." This reverts commit 013e47be09cfe0bbd5d2c6e95cda91d3c8e1e523. Reason for revert: This will be relanded as a warning, although the bugs which the original change exposed should be fixed soon. Original change's description: > Revert "Fix not-run-test detection." > > This reverts commit 41691abe86bc361799737a63e06eb0f8dfb05e0a. > > Reason for revert: I thought https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3683378 would solve the problem but turns out this is the issue where it now reports it as a critical error. > > Original change's description: > > Fix not-run-test detection. > > > > In crrev.com/c/2986400 the skip_shebang_check flag was added to address > > the problem of some tests being run neither on Python 2 or Python 3. At > > the same time reporting was added to warn if tests were run on neither > > Python version. > > > > Unfortunately the reporting code was incorrect. A PresubmitPromptWarning > > object was created but was not added to the list of results, so it had > > no effect. Also the skipped tests name was added where a list was > > expected which meant that each character of the test name showed up on > > its own line. > > > > This change fixes the reporting. It also changes the format of the > > report and makes it an error - a failure to run tests at all deserves > > that severity level. > > > > A test run with the fixed check showed that no errors have crept in - > > all tests were being run with one or the other. However the fix did > > find a bug in an in-progress change I was working on. > > > > Bug: 1223478 > > Change-Id: Ibb44b5e60e7a7a5de08302f19ee4035cdfac5212 > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3674199 > > Auto-Submit: Bruce Dawson > > Commit-Queue: Fabrice de Gans > > Reviewed-by: Fabrice de Gans > > Bug: 1223478 > Change-Id: If4395e90784c3f91692f398b4e5230a51139f78a > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3683105 > Commit-Queue: Bruce Dawson > Bot-Commit: Rubber Stamper > Reviewed-by: Bruce Dawson Bug: 1223478, 1330859 Change-Id: I6a0a78e70db77c9c5fda5bab27960cd4e71279e5 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3685349 Reviewed-by: Fabrice de Gans Commit-Queue: Bruce Dawson --- presubmit_canned_checks.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py index 314a8cb8a..5131d74cb 100644 --- a/presubmit_canned_checks.py +++ b/presubmit_canned_checks.py @@ -835,10 +835,9 @@ def GetUnitTests(input_api, message=message_type)) test_run = True if not test_run: - output_api.PresubmitPromptWarning( - "Some python tests were not run. You may need to add\n" - "skip_shebang_check=True for python3 tests.", - items=unit_test) + results.append(output_api.PresubmitPromptWarning( + "The %s test was not run. You may need to add\n" + "skip_shebang_check=True for python3 tests." % unit_test)) return results