mirror of
https://github.com/dart-lang/sdk
synced 2024-10-14 11:58:13 +00:00
Revert "Require that all changes to VM have TEST line"
This reverts commit a2ceec3e25
.
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 <vegorov@google.com>
> Reviewed-by: Martin Kustermann <kustermann@google.com>
> Reviewed-by: Siva Annamalai <asiva@google.com>
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 <vegorov@google.com>
Commit-Queue: Vyacheslav Egorov <vegorov@google.com>
This commit is contained in:
parent
a5afc3e974
commit
4b6f0f5c7b
42
PRESUBMIT.py
42
PRESUBMIT.py
|
@ -262,47 +262,6 @@ def _CheckLayering(input_api, output_api):
|
||||||
return []
|
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):
|
def _CheckClangTidy(input_api, output_api):
|
||||||
"""Run clang-tidy on VM changes."""
|
"""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(_CheckLayering(input_api, output_api))
|
||||||
results.extend(_CheckClangTidy(input_api, output_api))
|
results.extend(_CheckClangTidy(input_api, output_api))
|
||||||
results.extend(_CheckTestMatrixValid(input_api, output_api))
|
results.extend(_CheckTestMatrixValid(input_api, output_api))
|
||||||
results.extend(_CheckTestPlan(input_api, output_api))
|
|
||||||
results.extend(
|
results.extend(
|
||||||
input_api.canned_checks.CheckPatchFormatted(input_api, output_api))
|
input_api.canned_checks.CheckPatchFormatted(input_api, output_api))
|
||||||
return results
|
return results
|
||||||
|
|
Loading…
Reference in a new issue