diff mbox series

[RESEND,06/10] test: environment in ext4

Message ID 20200212184501.5911-7-patrick.delaunay@st.com
State Superseded
Delegated to: Tom Rini
Headers show
Series env: ext4: add test for env in ext4 | expand

Commit Message

Patrick DELAUNAY Feb. 12, 2020, 6:44 p.m. UTC 
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.

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

 test/py/tests/test_env.py | 87 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 87 insertions(+)

Comments

Stephen Warren Feb. 19, 2020, 9:45 p.m. UTC | #1 
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.
diff mbox series

Patch

diff --git a/test/py/tests/test_env.py b/test/py/tests/test_env.py
index cbdb41031c..d35ad888a7 100644
--- a/test/py/tests/test_env.py
+++ b/test/py/tests/test_env.py
@@ -4,6 +4,10 @@ 
 
 # Test operation of shell commands relating to environment variables.
 
+import os
+import os.path
+from subprocess import call, check_call
+
 import pytest
 import u_boot_utils
 
@@ -380,3 +384,86 @@  def test_env_info_quiet(state_test_env):
     assert response == ""
     response = c.run_command('echo $?')
     assert response == "1"
+
+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
+    fs_img = c.config.result_dir  + '/' + filename
+
+    if os.path.exists(persistent):
+        c.log.action('Disk image file ' + persistent + ' already exists')
+    else:
+        try:
+            check_call('rm -f %s' % persistent, shell=True)
+            check_call('dd if=/dev/zero of=%s bs=1M count=16'
+                % persistent, shell=True)
+            check_call('mkfs.ext4 -O ^metadata_csum %s' % persistent, shell=True)
+        except CalledProcessError:
+            call('rm -f %s' % persistent, shell=True)
+            raise
+
+    call('cp -f %s %s' % (persistent, fs_img), shell=True)
+    return fs_img
+
+@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
+
+    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)
+    """
+    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
+
+    response = c.run_command('ext4ls host 0:0')
+    assert '8192 uboot.env' in response
+
+    response = c.run_command('env info')
+    assert 'env_valid = valid' in response
+    assert 'env_ready = true' in response
+    assert 'env_use_default = false' in response
+
+    response = c.run_command('env info -p -d')
+    assert 'Environment was loaded from persistent storage' in response
+    assert 'Environment can be persisted' in response
+
+    response = c.run_command('env info -d -q')
+    assert response == ""
+    response = c.run_command('echo $?')
+    assert response == "1"
+
+    response = c.run_command('env info -p -q')
+    assert response == ""
+    response = c.run_command('echo $?')
+    assert response == "0"
+
+    """ restore env_location: ENVL_NOWHERE (12)
+    """
+    c.run_command('env_loc 12')
+
+    response = c.run_command('env info')
+    assert 'env_valid = valid' in response
+    assert 'env_ready = true' in response
+    assert 'env_use_default = true' in response
+
+    response = c.run_command('env info -p -d')
+    assert 'Default environment is used' in response
+    assert 'Environment cannot be persisted' in response
+
+    call('rm -f %s' % fs_img, shell=True)