Clean up stacked-changes dogfood

This removes logspam from `git cl` when a user (like me) hasn't opted
either in or out. DOGFOOD_STACKED_CHANGES=1 is the codified behavior.
This removes the DOGFOOD_STACKED_CHANGES=0 opt-out.

Bug: b/265929888
Change-Id: Ia159f898512d815e3a4b76cc8859c599016a7bec
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/5274266
Auto-Submit: Peter Boström <pbos@chromium.org>
Commit-Queue: Joanna Wang <jojwang@chromium.org>
Reviewed-by: Joanna Wang <jojwang@chromium.org>
changes/66/5274266/4
Peter Boström 2 years ago committed by LUCI CQ
parent 784db7a5f0
commit 74a6ca92bb

@ -161,10 +161,6 @@ assert len(_KNOWN_GERRIT_TO_SHORT_URLS) == len(
# at once. Picked arbitrarily.
_MAX_STACKED_BRANCHES_UPLOAD = 20
# Environment variable to indicate if user is participating in the stcked
# changes dogfood.
DOGFOOD_STACKED_CHANGES_VAR = 'DOGFOOD_STACKED_CHANGES'
class GitPushError(Exception):
pass
@ -5061,31 +5057,7 @@ def CMDupload(parser, args):
print('No previous patchsets, so --retry-failed has no effect.')
options.retry_failed = False
disable_dogfood_stacked_changes = os.environ.get(
DOGFOOD_STACKED_CHANGES_VAR) == '0'
dogfood_stacked_changes = os.environ.get(DOGFOOD_STACKED_CHANGES_VAR) == '1'
# Only print message for folks who don't have DOGFOOD_STACKED_CHANGES set
# to an expected value.
if (options.squash and not dogfood_stacked_changes
and not disable_dogfood_stacked_changes):
print(
'This repo has been enrolled in the stacked changes dogfood.\n'
'`git cl upload` now uploads the current branch and all upstream '
'branches that have un-uploaded updates.\n'
'Patches can now be reapplied with --force:\n'
'`git cl patch --reapply --force`.\n'
'Googlers may visit http://go/stacked-changes-dogfood for more information.\n'
'\n'
'Depot Tools no longer sets new uploads to "WIP". Please update the\n'
'"Set new changes to "work in progress" by default" checkbox at\n'
'https://%s/settings/\n'
'\n'
'To opt-out use `export DOGFOOD_STACKED_CHANGES=0`.\n'
'To hide this message use `export DOGFOOD_STACKED_CHANGES=1`.\n'
'File bugs at https://bit.ly/3Y6opoI\n' % cl.GetGerritHost())
if options.squash and not disable_dogfood_stacked_changes:
if options.squash:
if options.cherry_pick_stacked:
try:
orig_args.remove('--cherry-pick-stacked')
@ -5100,12 +5072,10 @@ def CMDupload(parser, args):
# we force code path that will read issue from the config.
cl.lookedup_issue = False
return upload_branch_deps(cl, orig_args, options.force)
return 0
if options.cherry_pick_stacked:
parser.error(
'--cherry-pick-stacked is not available for this workflow.')
'--cherry-pick-stacked is not available without squash=true,')
# cl.GetMostRecentPatchset uses cached information, and can return the last
# patchset before upload. Calling it here makes it clear that it's the

@ -24,9 +24,6 @@ UPLOAD_SCRIPT = os.path.join(DEPOT_TOOLS, 'upload_metrics.py')
DEFAULT_COUNTDOWN = 10
# TODO(b/265929888): Remove this variable when dogfood is over.
DEPOT_TOOLS_ENV = ['DOGFOOD_STACKED_CHANGES']
INVALID_CONFIG_WARNING = (
'WARNING: Your metrics.cfg file was invalid or nonexistent. A new one will '
'be created.')
@ -221,12 +218,6 @@ class MetricsCollector(object):
self._collecting_metrics = True
self.add('metrics_version', metrics_utils.CURRENT_VERSION)
self.add('command', command_name)
for env in DEPOT_TOOLS_ENV:
if env in os.environ:
self.add_repeated('env_vars', {
'name': env,
'value': os.environ.get(env)
})
try:
start = time.time()

@ -1279,12 +1279,6 @@ class TestGitCl(unittest.TestCase):
change_id='Ixxx',
gitcookies_exists=False)
def test_gerrit_upload_without_change_id(self):
self.skipTest("flaky test - depends on DOGFOOD_STACKED_CHANGES=0")
self._run_gerrit_upload_test([],
'desc ✔\n\nBUG=\n\nChange-Id: Ixxx', [],
change_id='Ixxx')
def test_gerrit_upload_without_change_id_nosquash(self):
self._run_gerrit_upload_test(
['--no-squash'],
@ -1362,189 +1356,6 @@ class TestGitCl(unittest.TestCase):
final_description=(
'desc ✔\n\nBUG=\nR=foo@example.com\n\nChange-Id: I123456789'))
def test_gerrit_upload_force_sets_bug(self):
self.skipTest("flaky test - depends on DOGFOOD_STACKED_CHANGES=0")
self._run_gerrit_upload_test(
['-b', '10000', '-f'],
u'desc=\n\nBug: 10000\nChange-Id: Ixxx', [],
force=True,
fetched_description='desc=\n\nChange-Id: Ixxx',
change_id='Ixxx')
def test_gerrit_upload_corrects_wrong_change_id(self):
self.skipTest("flaky test - depends on DOGFOOD_STACKED_CHANGES=0")
self._run_gerrit_upload_test(
['-b', '10000', '-m', 'Title', '--edit-description'],
u'desc=\n\nBug: 10000\nChange-Id: Ixxxx', [],
issue='123456',
edit_description='desc=\n\nBug: 10000\nChange-Id: Izzzz',
fetched_description='desc=\n\nChange-Id: Ixxxx',
title='Title',
change_id='Ixxxx')
def test_gerrit_upload_force_sets_fixed(self):
self.skipTest("flaky test - depends on DOGFOOD_STACKED_CHANGES=0")
self._run_gerrit_upload_test(
['-x', '10000', '-f'],
u'desc=\n\nFixed: 10000\nChange-Id: Ixxx', [],
force=True,
fetched_description='desc=\n\nChange-Id: Ixxx',
change_id='Ixxx')
def test_gerrit_reviewer_multiple(self):
self.skipTest("flaky test - depends on DOGFOOD_STACKED_CHANGES=0")
self._run_gerrit_upload_test(
[], 'desc ✔\nBUG=\nR=another@example.com\n'
'CC=more@example.com,people@example.com\n\n'
'Change-Id: 123456789', ['another@example.com'],
cc=['more@example.com', 'people@example.com'],
change_id='123456789')
def test_gerrit_upload_squash_first_is_default(self):
self.skipTest("flaky test - depends on DOGFOOD_STACKED_CHANGES=0")
self._run_gerrit_upload_test([],
'desc ✔\nBUG=\n\nChange-Id: 123456789', [],
change_id='123456789')
def test_gerrit_upload_squash_first(self):
self.skipTest("flaky test - depends on DOGFOOD_STACKED_CHANGES=0")
self.skipTest("flaky test - depends on DOGFOOD_STACKED_CHANGES=0")
self._run_gerrit_upload_test(['--squash'],
'desc ✔\nBUG=\n\nChange-Id: 123456789', [],
squash=True,
change_id='123456789')
def test_gerrit_upload_squash_first_title(self):
self.skipTest("flaky test - depends on DOGFOOD_STACKED_CHANGES=0")
self.skipTest("flaky test - depends on DOGFOOD_STACKED_CHANGES=0")
self._run_gerrit_upload_test(['-f', '-t', 'title'],
'title\n\ndesc\n\nChange-Id: 123456789',
[],
title='title',
force=True,
squash=True,
log_description='desc',
change_id='123456789')
def test_gerrit_upload_squash_first_with_labels(self):
self.skipTest("flaky test - depends on DOGFOOD_STACKED_CHANGES=0")
self._run_gerrit_upload_test(
['--squash', '--cq-dry-run', '--enable-auto-submit'],
'desc ✔\nBUG=\n\nChange-Id: 123456789', [],
squash=True,
labels={
'Commit-Queue': 1,
'Auto-Submit': 1
},
change_id='123456789')
def test_gerrit_upload_squash_first_against_rev(self):
self.skipTest("flaky test - depends on DOGFOOD_STACKED_CHANGES=0")
custom_cl_base = 'custom_cl_base_rev_or_branch'
self._run_gerrit_upload_test(['--squash', custom_cl_base],
'desc ✔\nBUG=\n\nChange-Id: 123456789', [],
squash=True,
custom_cl_base=custom_cl_base,
change_id='123456789')
self.assertIn(
'If you proceed with upload, more than 1 CL may be created by Gerrit',
sys.stdout.getvalue())
def test_gerrit_upload_squash_reupload(self):
self.skipTest("flaky test - depends on DOGFOOD_STACKED_CHANGES=0")
description = 'desc ✔\nBUG=\n\nChange-Id: 123456789'
self._run_gerrit_upload_test(['--squash'],
description, [],
squash=True,
issue=123456,
change_id='123456789')
@mock.patch('sys.stderr', StringIO())
def test_gerrit_upload_squash_reupload_to_abandoned(self):
self.skipTest("flaky test - depends on DOGFOOD_STACKED_CHANGES=0")
description = 'desc ✔\nBUG=\n\nChange-Id: 123456789'
with self.assertRaises(SystemExitMock):
self._run_gerrit_upload_test(['--squash'],
description, [],
squash=True,
issue=123456,
fetched_status='ABANDONED',
change_id='123456789')
self.assertEqual(
'Change https://chromium-review.googlesource.com/123456 has been '
'abandoned, new uploads are not allowed\n', sys.stderr.getvalue())
@mock.patch('gerrit_util.GetAccountDetails',
return_value={'email': 'yet-another@example.com'})
def test_gerrit_upload_squash_reupload_to_not_owned(self, _mock):
self.skipTest("flaky test - depends on DOGFOOD_STACKED_CHANGES=0")
description = 'desc ✔\nBUG=\n\nChange-Id: 123456789'
self._run_gerrit_upload_test(['--squash'],
description, [],
squash=True,
issue=123456,
other_cl_owner='other@example.com',
change_id='123456789')
self.assertIn(
'WARNING: Change 123456 is owned by other@example.com, but you '
'authenticate to Gerrit as yet-another@example.com.\n'
'Uploading may fail due to lack of permissions',
sys.stdout.getvalue())
@mock.patch('sys.stderr', StringIO())
def test_gerrit_upload_for_merged(self):
self.skipTest("flaky test - depends on DOGFOOD_STACKED_CHANGES=0")
with self.assertRaises(SystemExitMock):
self._run_gerrit_upload_test(
[],
'desc ✔\n\nBUG=\n\nChange-Id: I123456789', [],
issue=123456,
fetched_status='MERGED',
change_id='I123456789',
reset_issue=False)
self.assertEqual('New uploads are not allowed.\n',
sys.stderr.getvalue())
def test_gerrit_upload_new_issue_for_merged(self):
self.skipTest("flaky test - depends on DOGFOOD_STACKED_CHANGES=0")
self._run_gerrit_upload_test([],
'desc ✔\n\nBUG=\n\nChange-Id: I123456789',
[],
issue=123456,
fetched_status='MERGED',
change_id='I123456789',
reset_issue=True)
def test_upload_change_description_editor(self):
self.skipTest("flaky test - depends on DOGFOOD_STACKED_CHANGES=0")
fetched_description = 'foo\n\nChange-Id: 123456789'
description = 'bar\n\nChange-Id: 123456789'
self._run_gerrit_upload_test(['--squash', '--edit-description'],
description, [],
fetched_description=fetched_description,
squash=True,
issue=123456,
change_id='123456789',
edit_description=description)
@mock.patch('git_cl.Changelist._UpdateWithExternalChanges',
return_value='newparent')
def test_upload_change_uses_external_base(self, *_mocks):
self.skipTest("flaky test - depends on DOGFOOD_STACKED_CHANGES=0")
squash_hash = 'branch.main.' + git_cl.GERRIT_SQUASH_HASH_CONFIG_KEY
self.mockGit.config[squash_hash] = 'beef2'
self._run_gerrit_upload_test(
['--squash'],
'desc ✔\n\nBUG=\n\nChange-Id: 123456789',
[],
squash=True,
issue=123456,
change_id='123456789',
ref_to_push='beef2',
external_parent='newparent',
)
@mock.patch('git_cl.Changelist.GetGerritHost',
return_value='chromium-review.googlesource.com')
@mock.patch('git_cl.Changelist.GetRemoteBranch',
@ -4970,56 +4781,8 @@ class CMDUploadTestCase(CMDTestCaseBase):
mock.patch('git_cl.Settings.GetRoot', return_value='').start()
mock.patch('git_cl.Settings.GetSquashGerritUploads',
return_value=True).start()
mock.patch.dict(os.environ, {
git_cl.DOGFOOD_STACKED_CHANGES_VAR: "0"
}).start()
self.addCleanup(mock.patch.stopall)
def testWarmUpChangeDetailCache(self):
self.assertEqual(0, git_cl.main(['upload']))
gerrit_util.GetChangeDetail.assert_called_once_with(
'chromium-review.googlesource.com', 'depot_tools~123456',
frozenset([
'LABELS', 'CURRENT_REVISION', 'DETAILED_ACCOUNTS',
'CURRENT_COMMIT'
]))
def testUploadRetryFailed(self):
# This test mocks out the actual upload part, and just asserts that
# after upload, if --retry-failed is added, then the tool will fetch try
# jobs from the previous patchset and trigger the right builders on the
# latest patchset.
git_cl._fetch_tryjobs.side_effect = [
# Latest patchset: No builds.
[],
# Patchset before latest: Some builds.
[{
'id': str(100 + idx),
'builder': {
'project': 'chromium',
'bucket': 'try',
'builder': 'bot_' + status.lower(),
},
'createTime': '2019-10-09T08:00:0%d.854286Z' % (idx % 10),
'tags': [],
'status': status,
} for idx, status in enumerate(self._STATUSES)],
]
self.assertEqual(0, git_cl.main(['upload', '--retry-failed']))
self.assertEqual([
mock.call(mock.ANY, 'cr-buildbucket.appspot.com', patchset=7),
mock.call(mock.ANY, 'cr-buildbucket.appspot.com', patchset=6),
], git_cl._fetch_tryjobs.mock_calls)
expected_buckets = [
('chromium', 'try', 'bot_failure'),
('chromium', 'try', 'bot_infra_failure'),
]
git_cl._trigger_tryjobs.assert_called_once_with(mock.ANY,
expected_buckets,
mock.ANY, 8)
class MakeRequestsHelperTestCase(unittest.TestCase):
def exampleGerritChange(self):
return {

Loading…
Cancel
Save