[U-Boot] [PATCH] test/py: don't use mmc_rd config for other mmc tests

From: Stephen Warren swarren@nvidia.com
Fix test_mmc_dev(), test_mmc_rescan(), test_mmc_info() not to use the same configuration data that test_mmc_rd() does. Doing so causes the following issues:
* The new code uncondtionally expects certain keys to exist in the configuration data. These keys do not exist in existing configuration data since they were not previously required, and there was no notification re: a requirement to add these new keys. This causes test failures due to thrown exceptions when accessing the non-existent keys.
* The new tests logically operate on different objects. test_mmc_rd() operates on ranges of sectors on an MMC device (which may be the entire set of sectors of a device, or a part of a device), whereas all the new tests operate solely on entire devices. These are separate things, and it's entirely likely that the user will wish to runs the two types of tests on different sets of data; see the example configuration data that this commit adds. Ideally, the new tests would have been added to a separate Python file, since they aren' closely related to the existing tests.
FIXME: Marek, can you please replace the "???" in this patch with some reasonable looking data? Thanks.
Cc: Marek Vasut marek.vasut@gmail.com Fixes: 4ffec8cdf512 ("test/py: mmc: Add 'mmc info' test") Fixes: ce4b2cafa79c ("test/py: mmc: Add 'mmc rescan' test") Fixes: 86dfd152c917 ("test/py: mmc: Add 'mmc dev' test") Signed-off-by: Stephen Warren swarren@nvidia.com --- test/py/tests/test_mmc_rd.py | 85 ++++++++++++++++++++++++++++-------- 1 file changed, 66 insertions(+), 19 deletions(-)
diff --git a/test/py/tests/test_mmc_rd.py b/test/py/tests/test_mmc_rd.py index 2dc715bb5101..a25aa5f6f78e 100644 --- a/test/py/tests/test_mmc_rd.py +++ b/test/py/tests/test_mmc_rd.py @@ -13,6 +13,53 @@ import u_boot_utils This test relies on boardenv_* to containing configuration values to define which MMC devices should be tested. For example:
+# Configuration data for test_mmc_dev, test_mmc_rescan, test_mmc_info; defines +# whole MMC devices that mmc dev/rescan/info commands may operate upon. +env__mmc_dev_configs = ( + { + 'fixture_id': 'emmc-boot0', + 'is_emmc': True, + 'devid': 0, + 'partid': 1, + 'info_device': ???, + 'info_speed': ???, + 'info_mode': ???, + 'info_buswidth': ???. + }, + { + 'fixture_id': 'emmc-boot1', + 'is_emmc': True, + 'devid': 0, + 'partid': 2, + 'info_device': ???, + 'info_speed': ???, + 'info_mode': ???, + 'info_buswidth': ???. + }, + { + 'fixture_id': 'emmc-data', + 'is_emmc': True, + 'devid': 0, + 'partid': 0, + 'info_device': ???, + 'info_speed': ???, + 'info_mode': ???, + 'info_buswidth': ???. + }, + { + 'fixture_id': 'sd', + 'is_emmc': False, + 'devid': 1, + 'partid': None, + 'info_device': ???, + 'info_speed': ???, + 'info_mode': ???, + 'info_buswidth': ???. + }, +} + +# Configuration data for test_mmc_rd; defines regions of the MMC (entire +# devices, or ranges of sectors) which can be read: env__mmc_rd_configs = ( { 'fixture_id': 'emmc-boot0', @@ -85,12 +132,12 @@ def mmc_dev(u_boot_console, is_emmc, devid, partid): assert good_response in response
@pytest.mark.buildconfigspec('cmd_mmc') -def test_mmc_dev(u_boot_console, env__mmc_rd_config): +def test_mmc_dev(u_boot_console, env__mmc_dev_config): """Test the "mmc dev" command.
Args: u_boot_console: A U-Boot console connection. - env__mmc_rd_config: The single MMC configuration on which + env__mmc_dev_config: The single MMC configuration on which to run the test. See the file-level comment above for details of the format.
@@ -98,20 +145,20 @@ def test_mmc_dev(u_boot_console, env__mmc_rd_config): Nothing. """
- is_emmc = env__mmc_rd_config['is_emmc'] - devid = env__mmc_rd_config['devid'] - partid = env__mmc_rd_config.get('partid', 0) + is_emmc = env__mmc_dev_config['is_emmc'] + devid = env__mmc_dev_config['devid'] + partid = env__mmc_dev_config.get('partid', 0)
# Select MMC device mmc_dev(u_boot_console, is_emmc, devid, partid)
@pytest.mark.buildconfigspec('cmd_mmc') -def test_mmc_rescan(u_boot_console, env__mmc_rd_config): +def test_mmc_rescan(u_boot_console, env__mmc_dev_config): """Test the "mmc rescan" command.
Args: u_boot_console: A U-Boot console connection. - env__mmc_rd_config: The single MMC configuration on which + env__mmc_dev_config: The single MMC configuration on which to run the test. See the file-level comment above for details of the format.
@@ -119,9 +166,9 @@ def test_mmc_rescan(u_boot_console, env__mmc_rd_config): Nothing. """
- is_emmc = env__mmc_rd_config['is_emmc'] - devid = env__mmc_rd_config['devid'] - partid = env__mmc_rd_config.get('partid', 0) + is_emmc = env__mmc_dev_config['is_emmc'] + devid = env__mmc_dev_config['devid'] + partid = env__mmc_dev_config.get('partid', 0)
# Select MMC device mmc_dev(u_boot_console, is_emmc, devid, partid) @@ -132,12 +179,12 @@ def test_mmc_rescan(u_boot_console, env__mmc_rd_config): assert 'no card present' not in response
@pytest.mark.buildconfigspec('cmd_mmc') -def test_mmc_info(u_boot_console, env__mmc_rd_config): +def test_mmc_info(u_boot_console, env__mmc_dev_config): """Test the "mmc info" command.
Args: u_boot_console: A U-Boot console connection. - env__mmc_rd_config: The single MMC configuration on which + env__mmc_dev_config: The single MMC configuration on which to run the test. See the file-level comment above for details of the format.
@@ -145,13 +192,13 @@ def test_mmc_info(u_boot_console, env__mmc_rd_config): Nothing. """
- is_emmc = env__mmc_rd_config['is_emmc'] - devid = env__mmc_rd_config['devid'] - partid = env__mmc_rd_config.get('partid', 0) - info_device = env__mmc_rd_config['info_device'] - info_speed = env__mmc_rd_config['info_speed'] - info_mode = env__mmc_rd_config['info_mode'] - info_buswidth = env__mmc_rd_config['info_buswidth'] + is_emmc = env__mmc_dev_config['is_emmc'] + devid = env__mmc_dev_config['devid'] + partid = env__mmc_dev_config.get('partid', 0) + info_device = env__mmc_dev_config['info_device'] + info_speed = env__mmc_dev_config['info_speed'] + info_mode = env__mmc_dev_config['info_mode'] + info_buswidth = env__mmc_dev_config['info_buswidth']
# Select MMC device mmc_dev(u_boot_console, is_emmc, devid, partid)

On 4/16/19 4:04 PM, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
Fix test_mmc_dev(), test_mmc_rescan(), test_mmc_info() not to use the same configuration data that test_mmc_rd() does. Doing so causes the following issues:
- The new code uncondtionally expects certain keys to exist in the
configuration data. These keys do not exist in existing configuration data since they were not previously required, and there was no notification re: a requirement to add these new keys. This causes test failures due to thrown exceptions when accessing the non-existent keys.
- The new tests logically operate on different objects. test_mmc_rd()
operates on ranges of sectors on an MMC device (which may be the entire set of sectors of a device, or a part of a device), whereas all the new tests operate solely on entire devices. These are separate things, and it's entirely likely that the user will wish to runs the two types of tests on different sets of data; see the example configuration data that this commit adds. Ideally, the new tests would have been added to a separate Python file, since they aren' closely related to the existing tests.
FIXME: Marek, can you please replace the "???" in this patch with some reasonable looking data? Thanks.
Tom, Marek, any chance of applying this? Right now, every mainline branch is failing 5 tests on every push, which wastes my time having to go through all the logs to manually check that the failures aren't anything new/unknown. Thanks.

On 4/30/19 5:29 PM, Stephen Warren wrote:
On 4/16/19 4:04 PM, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
Fix test_mmc_dev(), test_mmc_rescan(), test_mmc_info() not to use the same configuration data that test_mmc_rd() does. Doing so causes the following issues:
- The new code uncondtionally expects certain keys to exist in the
configuration data. These keys do not exist in existing configuration data since they were not previously required, and there was no notification re: a requirement to add these new keys. This causes test failures due to thrown exceptions when accessing the non-existent keys.
- The new tests logically operate on different objects. test_mmc_rd()
operates on ranges of sectors on an MMC device (which may be the entire set of sectors of a device, or a part of a device), whereas all the new tests operate solely on entire devices. These are separate things, and it's entirely likely that the user will wish to runs the two types of tests on different sets of data; see the example configuration data that this commit adds. Ideally, the new tests would have been added to a separate Python file, since they aren' closely related to the existing tests.
FIXME: Marek, can you please replace the "???" in this patch with some reasonable looking data? Thanks.
Tom, Marek, any chance of applying this? Right now, every mainline branch is failing 5 tests on every push, which wastes my time having to go through all the logs to manually check that the failures aren't anything new/unknown. Thanks.
I'm still overloaded, and didn't have time to look into this. But I'm really not fond of the duplication here -- now we have two big tables describing very much the same thing, which SD/MMC to test .

On 4/30/19 10:27 AM, Marek Vasut wrote:
On 4/30/19 5:29 PM, Stephen Warren wrote:
On 4/16/19 4:04 PM, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
Fix test_mmc_dev(), test_mmc_rescan(), test_mmc_info() not to use the same configuration data that test_mmc_rd() does. Doing so causes the following issues:
- The new code uncondtionally expects certain keys to exist in the
configuration data. These keys do not exist in existing configuration data since they were not previously required, and there was no notification re: a requirement to add these new keys. This causes test failures due to thrown exceptions when accessing the non-existent keys.
- The new tests logically operate on different objects. test_mmc_rd()
operates on ranges of sectors on an MMC device (which may be the entire set of sectors of a device, or a part of a device), whereas all the new tests operate solely on entire devices. These are separate things, and it's entirely likely that the user will wish to runs the two types of tests on different sets of data; see the example configuration data that this commit adds. Ideally, the new tests would have been added to a separate Python file, since they aren' closely related to the existing tests.
FIXME: Marek, can you please replace the "???" in this patch with some reasonable looking data? Thanks.
Tom, Marek, any chance of applying this? Right now, every mainline branch is failing 5 tests on every push, which wastes my time having to go through all the logs to manually check that the failures aren't anything new/unknown. Thanks.
I'm still overloaded, and didn't have time to look into this. But I'm really not fond of the duplication here -- now we have two big tables describing very much the same thing, which SD/MMC to test .
There's no redundancy; the two different tests are semantically entirely different and don't share any configuration. See the patch description above for the full details.

On 5/1/19 4:51 PM, Stephen Warren wrote:
On 4/30/19 10:27 AM, Marek Vasut wrote:
On 4/30/19 5:29 PM, Stephen Warren wrote:
On 4/16/19 4:04 PM, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
Fix test_mmc_dev(), test_mmc_rescan(), test_mmc_info() not to use the same configuration data that test_mmc_rd() does. Doing so causes the following issues:
- The new code uncondtionally expects certain keys to exist in the
configuration data. These keys do not exist in existing configuration data since they were not previously required, and there was no notification re: a requirement to add these new keys. This causes test failures due to thrown exceptions when accessing the non-existent keys.
- The new tests logically operate on different objects. test_mmc_rd()
operates on ranges of sectors on an MMC device (which may be the entire set of sectors of a device, or a part of a device), whereas all the new tests operate solely on entire devices. These are separate things, and it's entirely likely that the user will wish to runs the two types of tests on different sets of data; see the example configuration data that this commit adds. Ideally, the new tests would have been added to a separate Python file, since they aren' closely related to the existing tests.
FIXME: Marek, can you please replace the "???" in this patch with some reasonable looking data? Thanks.
Tom, Marek, any chance of applying this? Right now, every mainline branch is failing 5 tests on every push, which wastes my time having to go through all the logs to manually check that the failures aren't anything new/unknown. Thanks.
I'm still overloaded, and didn't have time to look into this. But I'm really not fond of the duplication here -- now we have two big tables describing very much the same thing, which SD/MMC to test .
There's no redundancy; the two different tests are semantically entirely different and don't share any configuration. See the patch description above for the full details.
Tom, could you please apply this patch to fix the test failures. Thanks.

On Tue, Apr 16, 2019 at 04:04:49PM -0600, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
Fix test_mmc_dev(), test_mmc_rescan(), test_mmc_info() not to use the same configuration data that test_mmc_rd() does. Doing so causes the following issues:
- The new code uncondtionally expects certain keys to exist in the
configuration data. These keys do not exist in existing configuration data since they were not previously required, and there was no notification re: a requirement to add these new keys. This causes test failures due to thrown exceptions when accessing the non-existent keys.
- The new tests logically operate on different objects. test_mmc_rd()
operates on ranges of sectors on an MMC device (which may be the entire set of sectors of a device, or a part of a device), whereas all the new tests operate solely on entire devices. These are separate things, and it's entirely likely that the user will wish to runs the two types of tests on different sets of data; see the example configuration data that this commit adds. Ideally, the new tests would have been added to a separate Python file, since they aren' closely related to the existing tests.
FIXME: Marek, can you please replace the "???" in this patch with some reasonable looking data? Thanks.
Cc: Marek Vasut marek.vasut@gmail.com Fixes: 4ffec8cdf512 ("test/py: mmc: Add 'mmc info' test") Fixes: ce4b2cafa79c ("test/py: mmc: Add 'mmc rescan' test") Fixes: 86dfd152c917 ("test/py: mmc: Add 'mmc dev' test") Signed-off-by: Stephen Warren swarren@nvidia.com
Applied to u-boot/master, thanks!

On 5/10/19 5:12 AM, Tom Rini wrote:
On Tue, Apr 16, 2019 at 04:04:49PM -0600, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
Fix test_mmc_dev(), test_mmc_rescan(), test_mmc_info() not to use the same configuration data that test_mmc_rd() does. Doing so causes the following issues:
- The new code uncondtionally expects certain keys to exist in the
configuration data. These keys do not exist in existing configuration data since they were not previously required, and there was no notification re: a requirement to add these new keys. This causes test failures due to thrown exceptions when accessing the non-existent keys.
- The new tests logically operate on different objects. test_mmc_rd()
operates on ranges of sectors on an MMC device (which may be the entire set of sectors of a device, or a part of a device), whereas all the new tests operate solely on entire devices. These are separate things, and it's entirely likely that the user will wish to runs the two types of tests on different sets of data; see the example configuration data that this commit adds. Ideally, the new tests would have been added to a separate Python file, since they aren' closely related to the existing tests.
FIXME: Marek, can you please replace the "???" in this patch with some reasonable looking data? Thanks.
Cc: Marek Vasut marek.vasut@gmail.com Fixes: 4ffec8cdf512 ("test/py: mmc: Add 'mmc info' test") Fixes: ce4b2cafa79c ("test/py: mmc: Add 'mmc rescan' test") Fixes: 86dfd152c917 ("test/py: mmc: Add 'mmc dev' test") Signed-off-by: Stephen Warren swarren@nvidia.com
Applied to u-boot/master, thanks!
Thanks. Testing of u-boot/master is all back to green now.
participants (3)
-
Marek Vasut
-
Stephen Warren
-
Tom Rini