[U-Boot] [PATCH v1 1/3] test/py: gpt: update test_gpt

[Stephen Warren]

On 10/09/2017 01:47 AM, Patrick Delaunay wrote:
> - copy the reference gpt binary file as it is modified during the test
>    to avoid issue if the test fail: the test always restart with clean file
> - update tests to highlight detected issues on gpt swap command
>    (offset and size of partition are modified)
> - add test for gpt verfiy, gpt read and gpt write

It might be nice to split this into separate patches for separate 
logical changes.

> diff --git a/test/py/tests/test_gpt.py b/test/py/tests/test_gpt.py

>   These tests rely on a 4 MB disk image, which is automatically created by
> -the test.
> +the test, but as we expect specific content and it is modified by the gpt
> +commands executed during the test, it is not safe be reused it directly

That's a bit of an internal detail. I'm not sure it's necessary to spell 
it out in the documentation?

>   class GptTestDiskImage(object):
> @@ -28,26 +29,31 @@ class GptTestDiskImage(object):
>           """
>   
>           filename = 'test_gpt_disk_image.bin'
> +
>           self.path = u_boot_console.config.persistent_data_dir + '/' + filename
> +        ref_path = u_boot_console.config.persistent_data_dir + '/ref_' + filename

Can we put self.path somewhere other than persistent_data_dir? Since it 
gets re-created every time, it's not persistent. Other tests use 
u_boot_console.config.result_dir for temporary files.

> @@ -64,6 +70,30 @@ def state_disk_image(u_boot_console):
>   @pytest.mark.boardspec('sandbox')
>   @pytest.mark.buildconfigspec('cmd_gpt')
>   @pytest.mark.requiredtool('sgdisk')
> +def test_gpt_read(state_disk_image, u_boot_console):
> +    """Test the gpt read command."""
> +
> +    u_boot_console.run_command('host bind 0 ' + state_disk_image.path)
> +    output = u_boot_console.run_command('gpt read host 0')
> +    assert 'Start 1MiB, size 0MiB' in output
> +    assert 'Start 2MiB, size 0MiB' in output
> +    output = u_boot_console.run_command('part list host 0')

I think this test should also be:
@pytest.mark.buildconfigspec('cmd_part')

Some of the other diffs in this path add use of the part command for the 
first time, so I think you need to add that decorator to a few other 
functions too.

> @@ -109,9 +142,29 @@ def test_gpt_swap_partitions(state_disk_image, u_boot_console):
>   
>       u_boot_console.run_command('host bind 0 ' + state_disk_image.path)
>       output = u_boot_console.run_command('part list host 0')
> -    assert '0x000007ff	"first"' in output
> -    assert '0x000017ff	"second"' in output
> +    assert '0x00000800	0x00000a00	"first"' in output
> +    assert '0x00001000	0x00001200	"second"' in output

I'm not sure why this change is required. I can see two separate changes 
here:

1) Verifies the value of 3 columns of data not just 2. That seems fine.

2) Changes the expected value in the column before the partition name. 
I'm not sure about this; shouldn't the value stay the same, or is the 
test failing right now?