On 2/12/20 11:44 AM, Patrick Delaunay wrote:
Add basic test to persistent environment in ext4:
save and load in host ext4 file 'uboot.env'.

On first execution a 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):
+    c = state_test_env.u_boot_console
+
+    """Create a empty ext4 file system volume.
+    """
+    filename = 'env.ext4.img'
+    persistent = c.config.persistent_data_dir + '/' + filename

The test seems to want to delete this file at the end, and re-create it each time. Why store it in the persistent data directory in that case?

+    if os.path.exists(persistent):
+        c.log.action('Disk image file ' + persistent + ' already exists')

... and why just delete it and re-create it if a stale copy exists?

+    else:
+        try:
+            check_call('rm -f %s' % persistent, shell=True)

why not:

    check_call('rm -f ' + persistent)

That has simpler string operations, and I don't think any of these commands need shell=True, since they don't contain any shell metacharacters? Same comment for most of the other commands.

+def test_env_ext4(state_test_env):

+    """ env_location: ENVL_EXT4 (2)
+    """

I think that'd be better as a regular comment:

    # env_location: ENVL_EXT4 (2)

If there's a reason to make it a docstring, lets put the trailing """ on the same line since it's a short piece of text.

+    response = c.run_command('env_loc 2')
+    assert 'Saving Environment to EXT4' in response
+
+    response = c.run_command('env_loc 2')
+    assert 'Loading Environment from EXT4... OK' in response

Can't both assert invocations run on the same response? Or, does the env_loc command do different things based on some other state; that would feel pretty nasty.

Reply via email to