diff --git a/git_cl.py b/git_cl.py index debc7aa48e..74ece83abb 100755 --- a/git_cl.py +++ b/git_cl.py @@ -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 diff --git a/metrics.py b/metrics.py index 9ba1083d61..627db84aed 100644 --- a/metrics.py +++ b/metrics.py @@ -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() diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index 25e1ff5d52..8d7f4dc1ff 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -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 {