[PATCH v2 6/9] test: environment in ext4

[Patrick DELAUNAY]

Hi,

> From: Stephen Warren <swarren at wwwdotorg.org>
> Sent: lundi 22 juin 2020 20:57
> 
> On 6/16/20 1:40 AM, Patrick Delaunay wrote:
> > Add basic test to persistent environment in ext4:
> > save and load in host ext4 file 'uboot.env'.
> >
> > On first execution an empty EXT4 file system is created in persistent
> > data dir: env.ext4.img.
> 
> > diff --git a/test/py/tests/test_env.py b/test/py/tests/test_env.py
> 
> > +def mk_env_ext4(state_test_env):
> 
> > +    if os.path.exists(persistent):
> > +        c.log.action('Disk image file ' + persistent + ' already exists')
> > +    else:
> > +        try:
> > +            check_call('rm -f %s' % persistent, shell=True)
> 
> This should be run with the results logged to the overall test log file so that if there
> are failures, it's possible to see what they were. Use
> util.run_and_log() for this.

Ok, I will modifiy this part (I copy ext4 file create is copied form py/tests/test_fs/conftest.py:166 mk_fs() )

> Also, this particular command doesn't seem useful, since 4 lines above the code
> verified that the file doesn't exist.

Yes not needed, I remove this line.

> > + at pytest.mark.boardspec('sandbox')
> > + at pytest.mark.buildconfigspec('cmd_nvedit_info')
> > + at pytest.mark.buildconfigspec('cmd_echo')
> > + at pytest.mark.buildconfigspec('env_is_in_ext4')
> > +def test_env_ext4(state_test_env):
> > +
> > +    c = state_test_env.u_boot_console
> 
> Nit: That blank line is a bit odd.

OK
 
> > +    fs_img = mk_env_ext4(state_test_env)
> > +    c.run_command('host bind 0  %s' % fs_img)
> > +
> > +    response = c.run_command('ext4ls host 0:0')
> > +    assert 'uboot.env' not in response
> > +
> > +    """ env location: ENVL_EXT4 (2)
> > +    """
> 
> Nit: Wrap the trailing """ onto the same line; no need to force it to be a multi-line
> string. Also a comman may be better rather than a docstring. Same for the other
> docstring later.

OK, I don't realized that it was docstring.
 
> > +    call('rm -f %s' % fs_img, shell=True)
> 
> This won't happen if the test fails earlier. Should there be a try/except block or
> wrapper function with exception handling to resolve this?

Yes, I add it for V3

Patrick