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?
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to