languages/dart-sdk
languages/dart-sdk
1
0
Fork 0
mirror of https://github.com/dart-lang/sdk synced 2024-10-06 18:38:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

Require that all changes to VM have TEST line

Browse source 
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 <vegorov@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Siva Annamalai <asiva@google.com>
This commit is contained in:
Vyacheslav Egorov 2020-10-30 09:16:41 +00:00 committed by commit-bot@chromium.org
parent 2d4a3bd913
commit a2ceec3e25
1 changed files with 42 additions and 0 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

42
PRESUBMIT.py
View file

@ -262,6 +262,47 @@ 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."""
@ -333,6 +374,7 @@ 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