[tryserver] Report affected files via property
To enable aggregate analysis on changed files in BigQuery, conditionally
report them via an output property.
This CL causes a non-trivial roll only because of the sorting.
Recipe-Nontrivial-Roll: build
Bug: 1151655
Change-Id: Ie7a6c622196143c1aef07af9a9a356fb4202ea56
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2553349
Reviewed-by: Andrii Shyshkalov <tandrii@google.com>
Commit-Queue: Nodir Turakulov <nodir@chromium.org>
diff --git a/recipes/README.recipes.md b/recipes/README.recipes.md
index da747bb..59fc533 100644
--- a/recipes/README.recipes.md
+++ b/recipes/README.recipes.md
@@ -828,21 +828,23 @@
Populated iff gerrit_change is populated.
-— **def [get\_files\_affected\_by\_patch](/recipes/recipe_modules/tryserver/api.py#143)(self, patch_root, \*\*kwargs):**
+— **def [get\_files\_affected\_by\_patch](/recipes/recipe_modules/tryserver/api.py#143)(self, patch_root, report_files_via_property=None, \*\*kwargs):**
Returns list of paths to files affected by the patch.
Args:
* patch_root: path relative to api.path['root'], usually obtained from
api.gclient.get_gerrit_patch_root().
+ * report_files_via_property: name of the output property to report the
+ list of the files. If None (default), do not report.
Returned paths will be relative to to patch_root.
-— **def [get\_footer](/recipes/recipe_modules/tryserver/api.py#253)(self, tag, patch_text=None):**
+— **def [get\_footer](/recipes/recipe_modules/tryserver/api.py#265)(self, tag, patch_text=None):**
Gets a specific tag from a CL description
-— **def [get\_footers](/recipes/recipe_modules/tryserver/api.py#233)(self, patch_text=None):**
+— **def [get\_footers](/recipes/recipe_modules/tryserver/api.py#245)(self, patch_text=None):**
Retrieves footers from the patch description.
@@ -861,20 +863,20 @@
Returns true iff we have a change to check out.
-— **def [normalize\_footer\_name](/recipes/recipe_modules/tryserver/api.py#257)(self, footer):**
+— **def [normalize\_footer\_name](/recipes/recipe_modules/tryserver/api.py#269)(self, footer):**
-— **def [set\_change](/recipes/recipe_modules/tryserver/api.py#260)(self, change):**
+— **def [set\_change](/recipes/recipe_modules/tryserver/api.py#272)(self, change):**
Set the gerrit change for this module.
Args:
* change: a self.m.buildbucket.common_pb2.GerritChange.
-— **def [set\_compile\_failure\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#196)(self):**
+— **def [set\_compile\_failure\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#208)(self):**
Mark the tryjob result as a compile failure.
-— **def [set\_invalid\_test\_results\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#208)(self):**
+— **def [set\_invalid\_test\_results\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#220)(self):**
Mark the tryjob result as having invalid test results.
@@ -882,32 +884,32 @@
(e.g. no list of specific test cases that failed, or too many
tests failing, etc).
-— **def [set\_patch\_failure\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#192)(self):**
+— **def [set\_patch\_failure\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#204)(self):**
Mark the tryjob result as failure to apply the patch.
-— **def [set\_subproject\_tag](/recipes/recipe_modules/tryserver/api.py#170)(self, subproject_tag):**
+— **def [set\_subproject\_tag](/recipes/recipe_modules/tryserver/api.py#182)(self, subproject_tag):**
Adds a subproject tag to the build.
This can be used to distinguish between builds that execute different steps
depending on what was patched, e.g. blink vs. pure chromium patches.
-— **def [set\_test\_expired\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#225)(self):**
+— **def [set\_test\_expired\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#237)(self):**
Mark the tryjob result as a test expiration.
This means a test task expired and was never scheduled, most likely due to
lack of capacity.
-— **def [set\_test\_failure\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#200)(self):**
+— **def [set\_test\_failure\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#212)(self):**
Mark the tryjob result as a test failure.
This means we started running actual tests (not prerequisite steps
like checkout or compile), and some of these tests have failed.
-— **def [set\_test\_timeout\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#217)(self):**
+— **def [set\_test\_timeout\_tryjob\_result](/recipes/recipe_modules/tryserver/api.py#229)(self):**
Mark the tryjob result as a test timeout.
diff --git a/recipes/recipe_modules/tryserver/api.py b/recipes/recipe_modules/tryserver/api.py
index b0d5513..cb06d3f 100644
--- a/recipes/recipe_modules/tryserver/api.py
+++ b/recipes/recipe_modules/tryserver/api.py
@@ -140,12 +140,16 @@
self.m.properties.get('patch_repo_url') and
self.m.properties.get('patch_ref'))
- def get_files_affected_by_patch(self, patch_root, **kwargs):
+ def get_files_affected_by_patch(self, patch_root,
+ report_files_via_property=None,
+ **kwargs):
"""Returns list of paths to files affected by the patch.
Args:
* patch_root: path relative to api.path['root'], usually obtained from
api.gclient.get_gerrit_patch_root().
+ * report_files_via_property: name of the output property to report the
+ list of the files. If None (default), do not report.
Returned paths will be relative to to patch_root.
"""
@@ -160,11 +164,19 @@
**kwargs)
paths = [self.m.path.join(patch_root, p) for p in
step_result.stdout.split()]
+ paths.sort()
if self.m.platform.is_win:
# Looks like "analyze" wants POSIX slashes even on Windows (since git
# uses that format even on Windows).
paths = [path.replace('\\', '/') for path in paths]
step_result.presentation.logs['files'] = paths
+ if report_files_via_property:
+ step_result.presentation.properties[report_files_via_property] = {
+ 'total_count': len(paths),
+ # Do not report too many because it might violate build size limits,
+ # and isn't very useful anyway.
+ 'first_100': paths[:100],
+ }
return paths
def set_subproject_tag(self, subproject_tag):
diff --git a/recipes/recipe_modules/tryserver/examples/full.expected/with_gerrit_patch.json b/recipes/recipe_modules/tryserver/examples/full.expected/with_gerrit_patch.json
index 1842a15..1ebd173 100644
--- a/recipes/recipe_modules/tryserver/examples/full.expected/with_gerrit_patch.json
+++ b/recipes/recipe_modules/tryserver/examples/full.expected/with_gerrit_patch.json
@@ -119,7 +119,8 @@
"name": "git diff to analyze patch",
"~followup_annotations": [
"@@@STEP_LOG_LINE@files@None/foo.cc@@@",
- "@@@STEP_LOG_END@files@@@"
+ "@@@STEP_LOG_END@files@@@",
+ "@@@SET_BUILD_PROPERTY@affected_files@{\"first_100\": [\"None/foo.cc\"], \"total_count\": 1}@@@"
]
},
{
diff --git a/recipes/recipe_modules/tryserver/examples/full.expected/with_gerrit_patch_and_target_ref.json b/recipes/recipe_modules/tryserver/examples/full.expected/with_gerrit_patch_and_target_ref.json
index 99cf747..c254f4f 100644
--- a/recipes/recipe_modules/tryserver/examples/full.expected/with_gerrit_patch_and_target_ref.json
+++ b/recipes/recipe_modules/tryserver/examples/full.expected/with_gerrit_patch_and_target_ref.json
@@ -119,7 +119,8 @@
"name": "git diff to analyze patch",
"~followup_annotations": [
"@@@STEP_LOG_LINE@files@None/foo.cc@@@",
- "@@@STEP_LOG_END@files@@@"
+ "@@@STEP_LOG_END@files@@@",
+ "@@@SET_BUILD_PROPERTY@affected_files@{\"first_100\": [\"None/foo.cc\"], \"total_count\": 1}@@@"
]
},
{
diff --git a/recipes/recipe_modules/tryserver/examples/full.expected/with_wrong_patch.json b/recipes/recipe_modules/tryserver/examples/full.expected/with_wrong_patch.json
index 7f7bb0f..9247615 100644
--- a/recipes/recipe_modules/tryserver/examples/full.expected/with_wrong_patch.json
+++ b/recipes/recipe_modules/tryserver/examples/full.expected/with_wrong_patch.json
@@ -12,7 +12,8 @@
"name": "git diff to analyze patch",
"~followup_annotations": [
"@@@STEP_LOG_LINE@files@foo.cc@@@",
- "@@@STEP_LOG_END@files@@@"
+ "@@@STEP_LOG_END@files@@@",
+ "@@@SET_BUILD_PROPERTY@affected_files@{\"first_100\": [\"foo.cc\"], \"total_count\": 1}@@@"
]
},
{
diff --git a/recipes/recipe_modules/tryserver/examples/full.expected/with_wrong_patch_new.json b/recipes/recipe_modules/tryserver/examples/full.expected/with_wrong_patch_new.json
index b67f83f..c5452bc 100644
--- a/recipes/recipe_modules/tryserver/examples/full.expected/with_wrong_patch_new.json
+++ b/recipes/recipe_modules/tryserver/examples/full.expected/with_wrong_patch_new.json
@@ -13,7 +13,8 @@
"name": "git diff to analyze patch",
"~followup_annotations": [
"@@@STEP_LOG_LINE@files@sub/project/foo.cc@@@",
- "@@@STEP_LOG_END@files@@@"
+ "@@@STEP_LOG_END@files@@@",
+ "@@@SET_BUILD_PROPERTY@affected_files@{\"first_100\": [\"sub/project/foo.cc\"], \"total_count\": 1}@@@"
]
},
{
diff --git a/recipes/recipe_modules/tryserver/examples/full.py b/recipes/recipe_modules/tryserver/examples/full.py
index 326394a..15c237c 100644
--- a/recipes/recipe_modules/tryserver/examples/full.py
+++ b/recipes/recipe_modules/tryserver/examples/full.py
@@ -39,7 +39,9 @@
if api.tryserver.is_gerrit_issue:
api.tryserver.get_footers()
api.tryserver.get_files_affected_by_patch(
- api.properties.get('test_patch_root'))
+ api.properties.get('test_patch_root'),
+ report_files_via_property='affected_files',
+ )
if api.tryserver.is_tryserver:
api.tryserver.set_subproject_tag('v8')
