Grant better control over bot_update test data revisions

Currently, in order to specify a revision for a project in the
bot_update output, one has to either set the fixed_revisions or the
revision_mapping values. Both of these have additional effects on the
output json (fixed_revisions or properties) and setting them requires
fully overriding the json output. This can easily lead to output data
that doesn't match the gclient config being used.

This change overhauls the output_json test API function by adding a
revisions parameter that can be used to override the revisions used for
a project without affecting other portions of the json output. A
mod_test_data-decorated revisions method was added that enables
overriding the revisions without having to specify the full output json,
so fixed_revisions and properties will be set automatically according to
the gclient config being used. The revision_mapping parameter was
renamed to revision_properties to better distinguish it from revisions
and more accurately indicate its contents.

The following changes in behavior occur for the default test data:
* The got_revision property will only be added if the gclient config being used doesn't specify a got_revision_reverse_mapping
* If a got_revision property is added because the gclient config doesn't specify a got_revision_reverse_mapping and commit_positions is True, a got_revision_cp property will also be added

Additionally a post_check_output_json method was added that simplifies
the process of making post checks on the contents of the bot_update json
output.

Change-Id: I012b440107fa3323564fa3e1e3024c038e9944a2
Recipe-Manual-Change: build
Recipe-Manual-Change: infra
Recipe-Nontrivial-Roll: build_internal
Recipe-Nontrivial-Roll: chromiumos
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/7152830
Reviewed-by: Yiwei Zhang <yiwzhang@google.com>
Commit-Queue: Garrett Beaty <gbeaty@google.com>
This commit is contained in:
Garrett Beaty
2025-11-17 12:11:54 -08:00
committed by LUCI CQ
parent 039035332b
commit a98a592d70
5 changed files with 225 additions and 71 deletions

View File

@@ -58,18 +58,18 @@
Recipe module to ensure a checkout is consistent on a bot.
#### **class [BotUpdateApi](/recipes/recipe_modules/bot_update/api.py#74)([RecipeApi][recipe_engine/wkt/RecipeApi]):**
#### **class [BotUpdateApi](/recipes/recipe_modules/bot_update/api.py#73)([RecipeApi][recipe_engine/wkt/RecipeApi]):**
&mdash; **def [\_\_call\_\_](/recipes/recipe_modules/bot_update/api.py#83)(self, name, cmd, \*\*kwargs):**
&mdash; **def [\_\_call\_\_](/recipes/recipe_modules/bot_update/api.py#82)(self, name, cmd, \*\*kwargs):**
Wrapper for easy calling of bot_update.
&mdash; **def [deapply\_patch](/recipes/recipe_modules/bot_update/api.py#683)(self, bot_update_result):**
&mdash; **def [deapply\_patch](/recipes/recipe_modules/bot_update/api.py#682)(self, bot_update_result):**
Deapplies a patch, taking care of DEPS and solution revisions properly.
&mdash; **def [ensure\_checkout](/recipes/recipe_modules/bot_update/api.py#194)(self, gclient_config=None, suffix=None, patch=True, update_presentation=True, patch_root=None, with_branch_heads=False, with_tags=False, no_fetch_tags=False, refs=None, clobber=False, root_solution_revision=None, gerrit_no_reset=False, gerrit_no_rebase_patch_ref=False, assert_one_gerrit_change=True, patch_refs=None, ignore_input_commit=False, add_blamelists=False, set_output_commit=False, step_test_data=None, enforce_fetch=False, download_topics=False, recipe_revision_overrides=None, step_tags=None, clean_ignored=False, \*\*kwargs):**
&mdash; **def [ensure\_checkout](/recipes/recipe_modules/bot_update/api.py#193)(self, gclient_config=None, suffix=None, patch=True, update_presentation=True, patch_root=None, with_branch_heads=False, with_tags=False, no_fetch_tags=False, refs=None, clobber=False, root_solution_revision=None, gerrit_no_reset=False, gerrit_no_rebase_patch_ref=False, assert_one_gerrit_change=True, patch_refs=None, ignore_input_commit=False, add_blamelists=False, set_output_commit=False, step_test_data=None, enforce_fetch=False, download_topics=False, recipe_revision_overrides=None, step_tags=None, clean_ignored=False, \*\*kwargs):**
Args:
* gclient_config: The gclient configuration to use when running bot_update.
@@ -107,7 +107,7 @@ Args:
* step_tags: a dict {tag name: tag value} of tags to add to the step
* clean_ignored: If True, also clean ignored files from the checkout.
&mdash; **def [get\_project\_revision\_properties](/recipes/recipe_modules/bot_update/api.py#660)(self, project_name, gclient_config=None):**
&mdash; **def [get\_project\_revision\_properties](/recipes/recipe_modules/bot_update/api.py#659)(self, project_name, gclient_config=None):**
Returns all property names used for storing the checked-out revision of
a given project.
@@ -121,14 +121,14 @@ Args:
Returns (list of str): All properties that'll hold the checked-out revision
of the given project. An empty list if no such properties exist.
&emsp; **@property**<br>&mdash; **def [last\_returned\_properties](/recipes/recipe_modules/bot_update/api.py#102)(self):**
&emsp; **@property**<br>&mdash; **def [last\_returned\_properties](/recipes/recipe_modules/bot_update/api.py#101)(self):**
&mdash; **def [resolve\_fixed\_revision](/recipes/recipe_modules/bot_update/api.py#611)(self, bot_update_result, name):**
&mdash; **def [resolve\_fixed\_revision](/recipes/recipe_modules/bot_update/api.py#610)(self, bot_update_result, name):**
Sets a fixed revision for a single dependency using project revision
properties.
&mdash; **def [step\_name](/recipes/recipe_modules/bot_update/api.py#700)(self, patch, suffix):**
&mdash; **def [step\_name](/recipes/recipe_modules/bot_update/api.py#699)(self, patch, suffix):**
### *recipe_modules* / [depot\_tools](/recipes/recipe_modules/depot_tools)
[DEPS](/recipes/recipe_modules/depot_tools/__init__.py#6): [recipe\_engine/cipd][recipe_engine/recipe_modules/cipd], [recipe\_engine/context][recipe_engine/recipe_modules/context], [recipe\_engine/platform][recipe_engine/recipe_modules/platform], [recipe\_engine/runtime][recipe_engine/recipe_modules/runtime]

View File

@@ -13,7 +13,6 @@ from recipe_engine.engine_types import StepPresentation
from PB.go.chromium.org.luci.buildbucket.proto import common as common_pb2
@dataclasses.dataclass(kw_only=True, frozen=True)
class RelativeRoot:
"""A root that is relative to the checkout root.
@@ -338,8 +337,8 @@ class BotUpdateApi(recipe_api.RecipeApi):
# Guarantee that first solution has a revision.
# TODO(machenbach): We should explicitly pass HEAD for ALL solutions
# that don't specify anything else.
first_sol = cfg.solutions[0].name
revisions[first_sol] = revisions.get(first_sol) or 'HEAD'
first_sln = cfg.solutions[0].name
revisions[first_sln] = revisions.get(first_sln) or 'HEAD'
if cfg.revisions:
# Only update with non-empty values. Some recipe might otherwise
@@ -347,7 +346,7 @@ class BotUpdateApi(recipe_api.RecipeApi):
revisions.update(
(k, v) for k, v in cfg.revisions.items() if v)
if cfg.solutions and root_solution_revision:
revisions[first_sol] = root_solution_revision
revisions[first_sln] = root_solution_revision
# Allow for overrides required to bisect into rolls.
revisions.update(self._deps_revision_overrides)
@@ -419,12 +418,12 @@ class BotUpdateApi(recipe_api.RecipeApi):
cmd.append('--clean-ignored')
# Inject Json output for testing.
first_sln = cfg.solutions[0].name
step_test_data = step_test_data or (lambda: self.test_api.output_json(
first_sln,
reverse_rev_map,
patch_root=patch_root,
first_sln=first_sln,
revisions=self._test_data.get('revisions', {}),
fixed_revisions=fixed_revisions,
got_revision_mapping=reverse_rev_map,
patch_root=patch_root,
fail_checkout=self._test_data.get('fail_checkout', False),
fail_patch=self._test_data.get('fail_patch', False),
commit_positions=self._test_data.get('commit_positions', True),

View File

@@ -135,8 +135,7 @@
"@@@STEP_LOG_LINE@json.output@ \"patch_root\": \"src\",@@@",
"@@@STEP_LOG_LINE@json.output@ \"properties\": {@@@",
"@@@STEP_LOG_LINE@json.output@ \"got_cr_revision\": \"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\",@@@",
"@@@STEP_LOG_LINE@json.output@ \"got_cr_revision_cp\": \"refs/heads/main@{#170242}\",@@@",
"@@@STEP_LOG_LINE@json.output@ \"got_revision\": \"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\"@@@",
"@@@STEP_LOG_LINE@json.output@ \"got_cr_revision_cp\": \"refs/heads/main@{#170242}\"@@@",
"@@@STEP_LOG_LINE@json.output@ },@@@",
"@@@STEP_LOG_LINE@json.output@ \"root\": \"src\",@@@",
"@@@STEP_LOG_LINE@json.output@ \"source_manifest\": {@@@",
@@ -160,8 +159,7 @@
"@@@STEP_LOG_LINE@json.output@}@@@",
"@@@STEP_LOG_END@json.output@@@",
"@@@SET_BUILD_PROPERTY@got_cr_revision@\"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\"@@@",
"@@@SET_BUILD_PROPERTY@got_cr_revision_cp@\"refs/heads/main@{#170242}\"@@@",
"@@@SET_BUILD_PROPERTY@got_revision@\"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\"@@@"
"@@@SET_BUILD_PROPERTY@got_cr_revision_cp@\"refs/heads/main@{#170242}\"@@@"
]
},
{
@@ -235,8 +233,7 @@
"@@@STEP_LOG_LINE@json.output@ \"patch_root\": \"src\",@@@",
"@@@STEP_LOG_LINE@json.output@ \"properties\": {@@@",
"@@@STEP_LOG_LINE@json.output@ \"got_cr_revision\": \"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\",@@@",
"@@@STEP_LOG_LINE@json.output@ \"got_cr_revision_cp\": \"refs/heads/main@{#170242}\",@@@",
"@@@STEP_LOG_LINE@json.output@ \"got_revision\": \"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\"@@@",
"@@@STEP_LOG_LINE@json.output@ \"got_cr_revision_cp\": \"refs/heads/main@{#170242}\"@@@",
"@@@STEP_LOG_LINE@json.output@ },@@@",
"@@@STEP_LOG_LINE@json.output@ \"root\": \"src\",@@@",
"@@@STEP_LOG_LINE@json.output@ \"source_manifest\": {@@@",

View File

@@ -100,6 +100,7 @@ def GenTests(api):
def ci_build(**kwargs):
kwargs.setdefault(
'git_repo', 'https://chromium.googlesource.com/chromium/src')
kwargs.setdefault('revision', '2d72510e447ab60a9728aeea2362d8be2cbd7789')
return (
api.buildbucket.ci_build('chromium/src', 'ci', 'linux', **kwargs) +
api.properties(patch=False)
@@ -262,10 +263,11 @@ def GenTests(api):
# attempts to do comparisons on the revision value
api.step_data(
'bot_update (without patch)',
api.bot_update.output_json(patch_root='src',
first_sln='src',
revision_mapping={'got_revision': 'src'},
commit_positions=False)))
api.bot_update.output_json(
patch_root='src',
first_sln='src',
got_revision_mapping={'got_revision': 'src'},
commit_positions=False)))
yield api.test(
'upload_traces',
@@ -342,3 +344,65 @@ def GenTests(api):
{'$depot_tools/bot_update': {
'stale_process_duration_override': 3000,
}}) + ci_build())
def check_revisions(check, output_json):
preconditions_valid = (
check(output_json['fixed_revisions']['infra'] == 'HEAD')
and check(output_json['fixed_revisions']['src'] ==
'2d72510e447ab60a9728aeea2362d8be2cbd7789'))
if not preconditions_valid:
return # pragma: no cover
manifest = output_json['manifest']
# An empty revision specified for a project where there is a fixed revision
# uses the fixed revision
check(manifest['src']['revision'] ==
'2d72510e447ab60a9728aeea2362d8be2cbd7789')
# An empty revision specified for a project where there is not a fixed
# revision will generate a revision for 'HEAD'
check(manifest['src/foo']['revision'] == api.bot_update.gen_revision(
'src/foo'))
# A non-empty revision specified for a project where there is not a fixed
# revision will use the revision (generating an actual revision for a ref)
check(manifest['src/v8']['revision'] == api.bot_update.gen_revision(
'src/v8@refs/bar'))
# A non-empty revision specified for a project where there is a fixed
# revision with the same value will use the revision (generating an actual
# revision for a ref)
check(manifest['infra']['revision'] == api.bot_update.gen_revision('infra'))
yield api.test(
'revisions',
ci_build(),
api.bot_update.revisions({
'src': '',
'src/foo': '',
'src/v8': 'refs/bar',
'infra': 'HEAD',
}),
api.bot_update.post_check_output_json('bot_update (without patch)',
check_revisions),
api.post_process(post_process.DropExpectation),
)
def check_revisions_overriding_generation(check, output_json):
# Precondition check
if not check(output_json['fixed_revisions']['infra'] == 'HEAD'):
return # pragma: no cover
manifest = output_json['manifest']
# A non-empty revision for a project where there is a fixed revision that is
# different is okay if the fixed revision would be replaced with a generated
# value: the specified revision will be used
check(manifest['infra']['revision'] == 'a' * 40)
yield api.test(
'revisions-overriding-revision-generation',
ci_build(),
api.bot_update.revisions({
'infra': 'a' * 40,
}),
api.bot_update.post_check_output_json(
'bot_update (without patch)', check_revisions_overriding_generation),
api.post_process(post_process.DropExpectation),
)

View File

@@ -4,6 +4,7 @@
import collections.abc
import hashlib
import json
import struct
import typing
@@ -12,6 +13,11 @@ from recipe_engine import recipe_test_api
class BotUpdateTestApi(recipe_test_api.RecipeTestApi):
@recipe_test_api.mod_test_data
@staticmethod
def revisions(val: dict[str, str]):
return val
@recipe_test_api.mod_test_data
@staticmethod
def fail_checkout(val: bool):
@@ -29,30 +35,59 @@ class BotUpdateTestApi(recipe_test_api.RecipeTestApi):
def output_json(
self,
first_sln: str,
revision_mapping: collections.abc.Mapping[str, str],
*,
patch_root: str | None = None,
first_sln: str | None = None,
revisions: collections.abc.Mapping[str, str] | None = None,
fixed_revisions: collections.abc.Mapping[str, str] | None = None,
got_revision_mapping: collections.abc.Mapping[str, str] | None = None,
patch_root: str | None = None,
fail_checkout: bool = False,
fail_patch: bool | typing.Literal['download'] = False,
commit_positions: bool = True,
):
"""Synthesized json output for bot_update.py.
Revision values will be replaced with generated values if they are
equal to 'HEAD' or if they are a ref (starts with refs/ or origin/).
Args:
first_sln: The name of the first solution in the gclient config.
revision_mapping: A mapping from property name to project
name/checkout-relative repo path. The resultant json will have
the given property set to the revision of that project.
patch_root: The relative path within the checkout to where the
patch should be applied.
This will apear in the resultant json's root value. If not
provided, then the first project that appears in revisions will
be used. An entry for this will appear in the resultant json's
manifest even if it doesn't appear in revisions, fixed_revisions
or got_revision_mapping.
revisions: A mapping from project name/checkout-relative repo path
to the revision that will appear in the manifests in the
resultant json. Any projects present in fixed_revisions or
got_revision_mapping will also result in entries in the
manifests. If non-empty, the resultant json's root value will be
the first project. An empty value for a project indicates to
generate a revision. If first_sln isn't specified, then the
first project will be used as first_sln. Dicts have a guaranteed
iteration order based on declaration/insertion order, but other
mappings may not, so they may produce inconsistent results if
first_sln isn't specified.
fixed_revisions: A mapping from project name/checkout-relative
repo path to the revisions to be used for the project. The
revisions can be either refs or commit hashes. The manifests in
the resultant json will have refs replaced with generated
revision values. The provided revisions will be reported as-is
in the fixed_revisions of the resultant json.
provided revisions will be reported as-is in the fixed_revisions
of the resultant json. The resultant json's manifests will
include entries for any project's not present in revisions. If
non-empty and revisions is empty or None, the resultant json's
root value will be the first project. It is an error if an entry
exists for a project in revisions unless the values are equal,
the value in revisions is empty or the value in fixed_revisions
would be replaced with a generated value and the value in
revisions would not be.
got_revision_mapping: A mapping from property name to project
name/checkout-relative repo path. The resultant json will have
the given property set to the revision of that project. The
resultant json's manifests will include entries for any projects
not present in revisions. If non-empty and revisions and
fixed_revisions are both empty or None, the resultant json's
root value will be the first project.
patch_root: The relative path within the checkout to where the
patch should be applied.
fail_checkout: Whether or not the simulated checkout should fail.
fail_patch: Whether or not the checkout should fail to apply the
patch. A value of 'download' can be provided to indicate a
@@ -69,44 +104,85 @@ class BotUpdateTestApi(recipe_test_api.RecipeTestApi):
'did_run': True,
}
fixed_revisions = fixed_revisions or {}
if fail_checkout:
t.retcode = 1
else:
revisions = {
project_name: self.gen_revision(project_name)
for project_name in set(revision_mapping.values())
}
if fixed_revisions:
for project_name, revision in fixed_revisions.items():
if revision == 'HEAD':
revision = self.gen_revision(project_name)
elif revision.startswith('refs/') or revision.startswith('origin/'):
revision = self.gen_revision('{}@{}'.format(project_name, revision))
revisions[project_name] = revision
assert revisions or fixed_revisions or got_revision_mapping, (
'a non-empty value must be provided for at least one of'
' revisions, fixed_revisions or got_revision_mapping')
assert first_sln or revisions, (
'a non-empty value must be provided for first_sln or revisions')
properties = {
property_name: revisions[project_name]
for property_name, project_name in revision_mapping.items()
}
properties.setdefault('got_revision', self.gen_revision(first_sln))
if commit_positions:
revisions = revisions or {}
first_sln = first_sln or next(iter(revisions))
fixed_revisions = fixed_revisions or {}
# If no got_revision_mapping is specified, then bot_update.py will set the
# got_revision property for the first solution. It also has some default
# properties mapped in GOT_REVISION_MAPPINGS that isn't handled here for
# test data
got_revision_mapping = got_revision_mapping or {'got_revision': first_sln}
def get_ref(project_name):
revision = fixed_revisions.get(project_name, 'HEAD')
if revision.startswith('origin/'):
return revision.replace('origin/', 'refs/heads/', 1)
if revision.startswith('refs/'):
def resolve_revision(project_name, revision):
if revision == 'HEAD':
return self.gen_revision(project_name)
if revision.startswith('refs/') or revision.startswith('origin/'):
return self.gen_revision('{}@{}'.format(project_name, revision))
return revision
def choose_revision(project_name):
fixed_revision = (fixed_revisions or {}).get(project_name)
assert fixed_revision is None or fixed_revision, (
f'empty fixed_revision provided for {project_name}')
revision = revisions.get(project_name)
match (revision, fixed_revision):
case ('', _):
return resolve_revision(project_name, fixed_revision or 'HEAD')
case (_, None):
return resolve_revision(project_name, revision)
case (_, _) if revision == fixed_revision:
return resolve_revision(project_name, revision)
case _, _:
def will_generate(rev):
return resolve_revision(project_name, rev) != rev
# If the revision and fixed_revision are different, then the fixed
# revision must be HEAD or a ref and revision must not be and the
# revision will be treated as the revision of the commit that HEAD
# or the ref point at
assert (
will_generate(fixed_revision) and not will_generate(revision)
), f'{project_name} revision and fixed_revision are different'
return revision
return 'refs/heads/main'
properties.update({
'%s_cp' % property_name:
('%s@{#%s}' %
(get_ref(project_name), self.gen_commit_position(project_name)))
for property_name, project_name in revision_mapping.items()
})
resolved_revisions = {
project_name: choose_revision(project_name)
for project_name in revisions
}
for project_name, fixed_revision in fixed_revisions.items():
if project_name not in resolved_revisions:
resolved_revisions[project_name] = resolve_revision(
project_name, fixed_revision)
resolved_revisions.setdefault(first_sln, self.gen_revision(first_sln))
properties = {}
for property_name, project_name in got_revision_mapping.items():
if project_name not in resolved_revisions:
resolved_revisions[project_name] = self.gen_revision(project_name)
properties[property_name] = resolved_revisions[project_name]
if commit_positions:
fixed_revision = (fixed_revisions.get(project_name)
or revisions.get(project_name) or 'HEAD')
if fixed_revision.startswith('origin/'):
ref = fixed_revision.replace('origin/', 'refs/heads/', 1)
elif fixed_revision.startswith('refs/'):
ref = fixed_revision
else:
ref = 'refs/heads/main'
properties[f'{property_name}_cp'] = '%s@{#%s}' % (
ref, self.gen_commit_position(project_name))
output.update({
'patch_root': patch_root,
@@ -121,7 +197,7 @@ class BotUpdateTestApi(recipe_test_api.RecipeTestApi):
'repository': 'https://fake.org/%s.git' % project_name,
'revision': revision,
}
for project_name, revision in revisions.items()
for project_name, revision in sorted(resolved_revisions.items())
}
})
@@ -135,7 +211,8 @@ class BotUpdateTestApi(recipe_test_api.RecipeTestApi):
'revision': revision
}
}
for project_name, revision in revisions.items()
for project_name, revision in sorted(
resolved_revisions.items())
}
}
})
@@ -179,3 +256,20 @@ class BotUpdateTestApi(recipe_test_api.RecipeTestApi):
"""Hash project to bogus deterministic Cr-Commit-Position values."""
h = hashlib.sha1(project.encode('utf-8'))
return struct.unpack('!I', h.digest()[:4])[0] % 300000
def post_check_output_json(self, step_name: str, custom_check_fn):
"""Perform a post check on the output json of a bot_update step.
Args:
step_name: The name of the bot_update step to check.
custom_check_fn: A function taking 2 positional arguments: the
"magic check" function provided by the recipe engine and the
deserialized bot_update output json.
"""
def perform_post_check(check, steps):
bot_update_step = steps[step_name]
output_json = json.loads(bot_update_step.logs['json.output'])
custom_check_fn(check, output_json)
return self.post_check(perform_post_check)