From 4b6f0f5c7b77108e9666f9a343ee9acc927ac4ea Mon Sep 17 00:00:00 2001 From: Vyacheslav Egorov Date: Fri, 30 Oct 2020 15:28:41 +0000 Subject: [PATCH] Revert "Require that all changes to VM have TEST line" This reverts commit a2ceec3e257112d7e77068489ccb50beb0da6c9f. Reason for revert: presubmit hooks are run very early (before CL description is edited), which breaks common workflows. Original change's description: > Require that all changes to VM have TEST line > > All changes touching one of the following directories will after this > change be required to contain TEST= line. > > runtime/vm > runtime/bin > runtime/lib > runtime/include > runtime/observatory > runtime/observatory_2 > runtime/platform > sdk/lib/_internal/vm > pkg/vm > > This line is supposed to describe in free form how change was validated, > for example by listing existing or newly added tests. > > The goal behind this requirement is to remind both reviewer and change > author that changes to the code base are in general expected to be > covered by tests, especially when CL is addressing a regression which > slipped through existing testing. > > Having TEST line in the description would allow both author and > reviewer to take additional time to consider if validation was > sufficient or additional test coverage is needed. > > The inspiration for this line comes from Chromium[1]. > > [1] https://chromium.googlesource.com/chromiumos/docs/+/master/contributing.md#describe-testing-performed > > TEST=changed file in runtime/vm and run git cl presubmit > > Change-Id: Ie16cf7c14af18e3a22a17084c0aebb4d1dfd6d23 > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/169640 > Commit-Queue: Vyacheslav Egorov > Reviewed-by: Martin Kustermann > Reviewed-by: Siva Annamalai TBR=vegorov@google.com,kustermann@google.com,asiva@google.com Change-Id: Ib2e198c322447e8d1166d5d05b2f3209d168f1e9 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/169887 Reviewed-by: Vyacheslav Egorov Commit-Queue: Vyacheslav Egorov --- PRESUBMIT.py | 42 ------------------------------------------ 1 file changed, 42 deletions(-) diff --git a/PRESUBMIT.py b/PRESUBMIT.py index 9d133af033b..bfaf1df881d 100644 --- a/PRESUBMIT.py +++ b/PRESUBMIT.py @@ -262,47 +262,6 @@ def _CheckLayering(input_api, output_api): return [] -def _CheckTestPlan(input_api, output_api): - """Run test plan check. - - Each change that touches specified directories in the checkout are required - to have TEST= line explaining how this change was validated. - """ - - DIRS = [ - 'runtime/vm', - 'runtime/bin', - 'runtime/lib', - 'runtime/include', - 'runtime/observatory', - 'runtime/observatory_2', - 'runtime/platform', - 'pkg/vm', - 'sdk/lib/_internal/vm', - ] - - # Run only if directory was affected. - files = [f.LocalPath() for f in input_api.AffectedFiles()] - affected = filter(lambda dir: any(f.startswith(dir) for f in files), DIRS) - if len(affected) != 0 and input_api.change.tags.get('TEST', '') == '': - return [ - output_api.PresubmitError('Change is missing TEST= line', - long_text=""" -When changing files in one of the following directories you -must include TEST= line at the end of your change description. - - %s - -This line is expected to explain in a free form the kind of testing -that was performed to validate effect of the change. For example, -it can list newly added tests or already existing tests which are assumed -to cover the change. -""" % (' '.join(affected))) - ] - - return [] - - def _CheckClangTidy(input_api, output_api): """Run clang-tidy on VM changes.""" @@ -374,7 +333,6 @@ def _CommonChecks(input_api, output_api): results.extend(_CheckLayering(input_api, output_api)) results.extend(_CheckClangTidy(input_api, output_api)) results.extend(_CheckTestMatrixValid(input_api, output_api)) - results.extend(_CheckTestPlan(input_api, output_api)) results.extend( input_api.canned_checks.CheckPatchFormatted(input_api, output_api)) return results