Hi, > From: Stephen Warren <swar...@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. > > +@pytest.mark.boardspec('sandbox') > > +@pytest.mark.buildconfigspec('cmd_nvedit_info') > > +@pytest.mark.buildconfigspec('cmd_echo') > > +@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