Merge pull request #54 from srfraser/verification2

Bug 1470232 Improving version bump safety

Author: Callek

treescript/exceptions.py

@@ -29,3 +29,17 @@ def __init__(self, msg):
         super(FailedSubprocess, self).__init__(
             msg, exit_code=STATUSES['internal-error']
         )
+
+
+class ChangesetMismatchError(ScriptWorkerTaskException):
+    """An incorrect number of changesets would be pushed."""
+
+    def __init__(self, msg):
+        """Initialize ChangesetMismatchError.
+
+        Args:
+            msg (str): the reason for throwing an exception.
+        """
+        super(ChangesetMismatchError, self).__init__(
+            msg, exit_code=STATUSES['internal-error']
+        )

treescript/mercurial.py

@@ -4,7 +4,7 @@
 import sys
 
 from treescript.utils import execute_subprocess
-from treescript.exceptions import FailedSubprocess
+from treescript.exceptions import FailedSubprocess, ChangesetMismatchError
 from treescript.task import get_source_repo, get_tag_info
 
 # https://www.mercurial-scm.org/repo/hg/file/tip/tests/run-tests.py#l1040
@@ -90,7 +90,7 @@ async def run_hg_command(context, *args, local_repo=None):
     env = build_hg_environment()
     if local_repo:
         command.extend(['-R', local_repo])
-    await execute_subprocess(command, env=env)
+    return await execute_subprocess(command, env=env)
 
 
 # log_mercurial_version {{{1
@@ -207,6 +207,23 @@ async def log_outgoing(context, directory):
     await run_hg_command(context, 'out', '-vp', '-r', '.', dest_repo, local_repo=local_repo)
 
 
+async def assert_outgoing(context, directory, expected_cset_count):
+    """Run `hg out` to ensure the expected number of changes exist.
+
+    Assuming that each action produces one changeset, we can
+    check to see if we'd push to the right location by comparing
+    the results of 'hg out' - if it's not equal to the number
+    of actions, we have unexpected changes to push.
+    """
+    local_repo = os.path.join(directory, 'src')
+    dest_repo = get_source_repo(context.task)
+    log.info("outgoing changesets..")
+    changesets = await run_hg_command(context, 'out', '-q', '-r', '.', dest_repo, local_repo=local_repo)
+    if len(changesets) != expected_cset_count:
+        raise ChangesetMismatchError(
+            'Expected {} changesets to push, found {}'.format(expected_cset_count, changesets))
+
+
 async def push(context):
     """Run `hg push` against the current source repo."""
     local_repo = context.repo

treescript/script.py

@@ -8,7 +8,7 @@
 from scriptworker.exceptions import ScriptWorkerException
 from treescript.utils import task_action_types, is_dry_run
 from treescript.mercurial import log_mercurial_version, validate_robustcheckout_works, \
-    checkout_repo, do_tagging, log_outgoing, push
+    checkout_repo, do_tagging, log_outgoing, assert_outgoing, push
 from treescript.versionmanip import bump_version
 
 log = logging.getLogger(__name__)
@@ -29,6 +29,8 @@ async def do_actions(context, actions, directory):
         else:
             raise NotImplementedError("Unexpected action")
     await log_outgoing(context, directory)
+    # 'push' doesn't generate a changeset, the other actions do.
+    await assert_outgoing(context, directory, len(actions)-actions.count('push'))
     if is_dry_run(context.task):
         log.info("Not pushing changes, dry_run was forced")
     elif 'push' in actions:

treescript/test/test_mercurial.py

@@ -11,7 +11,7 @@
 import pytest
 from scriptworker.context import Context
 from treescript import mercurial
-from treescript.exceptions import FailedSubprocess
+from treescript.exceptions import FailedSubprocess, ChangesetMismatchError
 from treescript.script import get_default_config
 from treescript.utils import mkdir
 from treescript.test import tmpdir, noop_async, is_slice_in_list
@@ -162,7 +162,8 @@ async def check_params(*args, **kwargs):
                                 args)
 
     mocker.patch.object(mercurial, 'run_hg_command', new=check_params)
-    mocker.patch.object(mercurial, 'get_source_repo').return_value = "https://hg.mozilla.org/test-repo"
+    mocker.patch.object(
+        mercurial, 'get_source_repo').return_value = "https://hg.mozilla.org/test-repo"
 
     context.config['hg_share_base_dir'] = '/builds/hg-shared-test'
     context.config['upstream_repo'] = 'https://hg.mozilla.org/mozilla-test-unified'
@@ -255,3 +256,30 @@ async def run_command(context, *arguments, local_repo=None):
     assert is_slice_in_list(('out', '-vp'), called_args[0][0])
     assert is_slice_in_list(('-r', '.'), called_args[0][0])
     assert is_slice_in_list(('https://hg.mozilla.org/treescript-test', ), called_args[0][0])
+
+
+@pytest.mark.asyncio
+async def test_assert_outgoing(context, mocker):
+    fake_csets = ['cset1', 'cset2']
+
+    async def dummy_run_hg_command(*args, **kwargs):
+        return fake_csets
+    mocker.patch.object(mercurial, 'run_hg_command', new=dummy_run_hg_command)
+
+    mocked_source_repo = mocker.patch.object(mercurial, 'get_source_repo')
+    mocked_source_repo.return_value = 'https://hg.mozilla.org/treescript-test'
+    await mercurial.assert_outgoing(context, context.config['work_dir'], len(fake_csets))
+
+
+@pytest.mark.asyncio
+async def test_assert_outgoing_failure(context, mocker):
+    fake_csets = ['cset1', 'cset2']
+
+    async def dummy_run_hg_command(*args, **kwargs):
+        return fake_csets + ['cset3']
+    mocker.patch.object(mercurial, 'run_hg_command', new=dummy_run_hg_command)
+
+    mocked_source_repo = mocker.patch.object(mercurial, 'get_source_repo')
+    mocked_source_repo.return_value = 'https://hg.mozilla.org/treescript-test'
+    with pytest.raises(ChangesetMismatchError):
+        await mercurial.assert_outgoing(context, context.config['work_dir'], len(fake_csets))

treescript/test/test_script.py

@@ -109,6 +109,7 @@ async def mocked_push(*args, **kwargs):
     mocker.patch.object(script, 'bump_version', new=mocked_bump)
     mocker.patch.object(script, 'push', new=mocked_push)
     mocker.patch.object(script, 'log_outgoing', new=noop_async)
+    mocker.patch.object(script, 'assert_outgoing', new=noop_async)
     mocker.patch.object(script, 'is_dry_run').return_value = dry_run
     await script.do_actions(context, actions, directory='/some/folder/here')
     assert called_tag[0]

treescript/test/test_utils.py

@@ -85,9 +85,9 @@ def test_is_dry_run_true():
     assert True is utils.is_dry_run(task)
 
 
-# log_output {{{1
+# process_output {{{1
 @pytest.mark.asyncio
-async def test_log_output(tmpdir, mocker):
+async def test_process_output(tmpdir, mocker):
     logged = []
     with open(__file__, 'r') as fh:
         contents = fh.read()
@@ -113,7 +113,7 @@ async def __anext__(self):
     mockfh = mock.MagicMock()
     aiter = AsyncIterator()
     mockfh.readline = aiter.__anext__
-    await utils.log_output(mockfh)
+    await utils.process_output(mockfh)
     assert contents.rstrip() == '\n'.join(logged)
 
 

treescript/utils.py

@@ -78,20 +78,24 @@ def is_dry_run(task):
     return dry_run
 
 
-# log_output {{{1
-async def log_output(fh):
+# process_output {{{1
+async def process_output(fh):
     """Log the output from an async generator.
 
     Args:
         fh (async generator): the async generator to log output from
 
     """
+    output = []
     while True:
         line = await fh.readline()
         if line:
-            log.info(line.decode("utf-8").rstrip())
+            line = line.decode("utf-8").rstrip()
+            log.info(line)
+            output.append(line)
         else:
             break
+    return output
 
 
 # execute_subprocess {{{1
@@ -114,9 +118,10 @@ async def execute_subprocess(command, **kwargs):
         *command, stdout=PIPE, stderr=STDOUT, **kwargs
     )
     log.info("COMMAND OUTPUT: ")
-    await log_output(subprocess.stdout)
+    output = await process_output(subprocess.stdout)
     exitcode = await subprocess.wait()
     log.info("exitcode {}".format(exitcode))
 
     if exitcode != 0:
         raise FailedSubprocess('Command `{}` failed'.format(' '.join(command)))
+    return output