curtin-dev team mailing list archive

[Chris Peterson <mp+460790@xxxxxxxxxxxxxxxxxx>]

Review: Approve

Very nice! One question about the kwargs bit but I looked through all the calls to run_bm and it doesn't look like it's really used anyways. 

Diff comments:

> diff --git a/tests/integration/test_block_meta.py b/tests/integration/test_block_meta.py
> index 2fc7c7d..1867f63 100644
> --- a/tests/integration/test_block_meta.py
> +++ b/tests/integration/test_block_meta.py
> @@ -267,7 +268,37 @@ class TestBlockMeta(IntegrationTestCase):
>              '-c', config_path, 'block-meta', '--testmode', 'custom',
>              *args,
>              ]
> -        util.subp(cmd, env=cmd_env, **kwargs)
> +
> +        # Set debug=True to halt the integration test and run curtin manually,
> +        # with the integration tests having setup the environment for you.
> +        # To see the script name run with "pytest-3 -s", or look at fp.name.
> +        if not kwargs.pop('debug', False):
> +            util.subp(cmd, env=cmd_env, **kwargs)
> +            return
> +
> +        env = cmd_env.copy()
> +        env.update(PYTHONPATH=os.getcwd())
> +        import pprint
> +        pp = pprint.PrettyPrinter(indent=4)
> +        code = '''\
> +#!/usr/bin/python3
> +import subprocess
> +cmd = {cmd}
> +env = {env}
> +subprocess.run(cmd, env=env)
> +'''.format(cmd=pp.pformat(cmd), env=pp.pformat(env))

Why not write the rest of kwargs to the script?

> +
> +        opts = dict(mode='w', delete=False, suffix='.py')
> +        with tempfile.NamedTemporaryFile(**opts) as fp:
> +            fp.write(code)
> +        try:
> +            os.chmod(fp.name, 0o700)
> +            print('\nThe integration test is paused.')
> +            print('Use script {} to run curtin manually.'.format(fp.name))
> +            import pdb
> +            pdb.set_trace()
> +        finally:
> +            os.unlink(fp.name)
>  
>      def _test_default_offsets(self, ptable, version, sector_size=512):
>          psize = 40 << 20


-- 
https://code.launchpad.net/~dbungert/curtin/+git/curtin/+merge/460790
Your team curtin developers is requested to review the proposed merge of ~dbungert/curtin:integration-run-bm-debug into curtin:master.