curtin-dev team mailing list archive

[Olivier Gayot <mp+443376@xxxxxxxxxxxxxxxxxx>]

A comment about the test helper script, but otherwise LGTM.

Diff comments:

> diff --git a/tests/integration/test_block_meta.py b/tests/integration/test_block_meta.py
> index d701ca0..d14a3f8 100644
> --- a/tests/integration/test_block_meta.py
> +++ b/tests/integration/test_block_meta.py
> @@ -257,7 +257,25 @@ 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.
> +        # Run tests with pytest-3 -s option.
> +        if kwargs.pop('debug', False):
> +            with tempfile.NamedTemporaryFile(mode='w', delete=False) as fp:
> +                fp.write('#!/bin/bash\n')
> +                for k, v in cmd_env.items():
> +                    fp.write('export {}="{}"\n'.format(k, v))

I'm always concerned about executing generated code, especially when bash and the environment are involved.

May I suggest the below, which hopefully minimizes the risk?

```
# Create a JSON file containing the environment to use and the command to run.
with tempfile.NamedTemporaryFile(mode='w', delete=False, suffix='.json') as metadata_fp:
    env = cmd_env.copy()
    env.update(PYTHONPATH=os.getcwd())
    json.dump({'env': cmd_env, 'cmd': cmd}, metadata_fp)

code = '''\
#!/usr/bin/env python3
import json
import subprocess

METADATA_FILE = {metadata_file}
with open(METADATA_FILE, mode="r") as fp:
   data = json.load(fp)

subprocess.run(data["cmd"], env=data["env"])
'''

with tempfile.NamedTemporaryFile(mode='w', delete=False, suffix='.py') as helper_fp:
    helper_fp.write(code.format(metadata_file=repr(metadata_fp.name)))
os.chmod(helper_fp.name, 0o700)
...

> +                fp.write('export PYTHONPATH="{}"\n'.format(os.getcwd()))
> +                fp.write(' '.join(cmd) + '\n')
> +            os.chmod(fp.name, 0o700)
> +            print()
> +            print('The integration test is paused.')
> +            print('Use script {} to run curtin manually.'.format(fp.name))
> +            breakpoint()
> +            os.unlink(fp.name)
> +        else:
> +            util.subp(cmd, env=cmd_env, **kwargs)
>  
>      def _test_default_offsets(self, ptable, version, sector_size=512):
>          psize = 40 << 20


-- 
https://code.launchpad.net/~dbungert/curtin/+git/curtin/+merge/443376
Your team curtin developers is subscribed to branch curtin:master.