Re: [PATCH v2 1/3] test: env: Add test for environment storage in SPI NOR

[Heinrich Schuchardt <xypron.glpk@gmx.de>]

On 12/23/25 15:31, Marek Vasut wrote:
> Add test for environment stored in SPI NOR. The test works in a very
> similar way to the current test for environment stored in ext4 FS,
> except it generates spi.bin file backing the SPI NOR.
> 
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> ---
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Cc: Jerome Forissier <jerome.forissier@linaro.org>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: u-boot@lists.denx.de
> ---
> V2: No change
> ---
>   test/py/tests/test_env.py | 99 +++++++++++++++++++++++++++++++++++++++
>   1 file changed, 99 insertions(+)
> 
> diff --git a/test/py/tests/test_env.py b/test/py/tests/test_env.py
> index 383e26c03b0..48e31f19b3c 100644
> --- a/test/py/tests/test_env.py
> +++ b/test/py/tests/test_env.py
> @@ -457,6 +457,26 @@ def mk_env_ext4(state_test_env):
>       utils.run_and_log(c, ['cp',  '-f', persistent, fs_img])
>       return fs_img
>   
> +def mk_env_spi_flash(state_test_env):

Thank you for adding the test.

Unfortunately pylint doesn't like your code. Please have a look.

W0621: Redefining name 'state_test_env' from outer scope (line 95) 
(redefined-outer-name)

> +
> +    """Create an empty SPI NOR image."""
> +    c = state_test_env.ubman
> +    filename = 'spi.bin'
> +    persistent = c.config.persistent_data_dir + '/' + filename
> +    spi_flash_img = c.config.source_dir  + '/' + filename
> +
> +    if os.path.exists(persistent):
> +        c.log.action('SPI NOR image file ' + persistent + ' already exists')
> +    else:
> +        try:
> +            utils.run_and_log(c, 'dd if=/dev/zero of=%s bs=1M count=2' % persistent)

C0209: Formatting a regular string which could be an f-string 
(consider-using-f-string)

> +        except CalledProcessError:
> +            call('rm -f %s' % persistent, shell=True)

C0209: Formatting a regular string which could be an f-string 
(consider-using-f-string)

Except for the pylint issues the patch looks good to me.

Best regards

Heinrich

> +            raise
> +
> +    utils.run_and_log(c, ['cp',  '-f', persistent, spi_flash_img])
> +    return spi_flash_img
> +
>   @pytest.mark.boardspec('sandbox')
>   @pytest.mark.buildconfigspec('cmd_echo')
>   @pytest.mark.buildconfigspec('cmd_nvedit_info')
> @@ -544,6 +564,85 @@ def test_env_ext4(state_test_env):
>           if fs_img:
>               call('rm -f %s' % fs_img, shell=True)
>   
> +@pytest.mark.boardspec('sandbox')
> +@pytest.mark.buildconfigspec('cmd_echo')
> +@pytest.mark.buildconfigspec('cmd_nvedit_info')
> +@pytest.mark.buildconfigspec('cmd_nvedit_load')
> +@pytest.mark.buildconfigspec('cmd_nvedit_select')
> +@pytest.mark.buildconfigspec('env_is_in_spi_flash')
> +def test_env_spi_flash(state_test_env):
> +
> +    """Test ENV in SPI NOR on sandbox."""
> +    c = state_test_env.ubman
> +    spi_flash_img = ''
> +    try:
> +        spi_flash_img = mk_env_spi_flash(state_test_env)
> +
> +        # force env location: SF
> +        response = c.run_command('env select SPIFlash')
> +        assert 'Select Environment on SPIFlash: OK' in response
> +
> +        response = c.run_command('env save')
> +        assert 'Saving Environment to SPIFlash' in response
> +
> +        response = c.run_command('env load')
> +        assert 'Loading Environment from SPIFlash... OK' 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"
> +
> +        response = c.run_command('env erase')
> +        assert 'OK' in response
> +
> +        response = c.run_command('env load')
> +        assert 'Loading Environment from SPIFlash... ' in response
> +        assert 'bad CRC, using default environment' in response
> +
> +        response = c.run_command('env info')
> +        assert 'env_valid = invalid' 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 can be persisted' in response
> +
> +        # restore env location: NOWHERE (prio 0 in sandbox)
> +        response = c.run_command('env select nowhere')
> +        assert 'Select Environment on nowhere: OK' in response
> +
> +        response = c.run_command('env load')
> +        assert 'Loading Environment from nowhere... OK' in response
> +
> +        response = c.run_command('env info')
> +        assert 'env_valid = invalid' 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
> +
> +    finally:
> +        if spi_flash_img:
> +            call('rm -f %s' % spi_flash_img, shell=True)
> +
>   def test_env_text(ubman):
>       """Test the script that converts the environment to a text file"""
>