Re: [U-Boot] [PATCH] test/py: Fix MMC/SD block write test dependency
On 15. 08. 19 17:42, Stephen Warren wrote: > On 8/15/19 12:26 AM, Michal Simek wrote: >> On 14. 08. 19 17:51, Stephen Warren wrote: >>> On 8/14/19 12:12 AM, Michal Simek wrote: On 13. 08. 19 23:21, Stephen Warren wrote: > On 8/1/19 10:48 PM, Michal Simek wrote: >> Test is using random command which has own Kconfig symbol CMD_RANDOM >> which >> already depends on CMD_MEMORY. That's why replace cmd_memory by >> cmd_random. > > This might not always be true; I think it'd be better to keep the > existing dependency list entries and just add CMD_RANDOM: Can you please tell me when exactly? I didn't see that. >>> >>> If in the future somebody changes the Kconfig file so that CMD_RANDOM >>> doesn't depend on or select CMD_MEMORY. It may well not be possible >>> right now; I just want to make sure the test doesn't rely on >>> implementation details of CMD_RANDOM/CMD_MEMORY; if it explicitly needs >>> both, it should simply depend on both. >> >> Do you think that we have used this rule in past? > > I hope so; I'd consider it a bug in a test if the test used a command > but didn't explicitly depend on that command's CONFIG_ flag. We have had > such bugs in the past, and fixed one or two of them. > >> Anyway I understand what you mean but it is questionable if make sense >> to predict all changes which can happen. > > It's more about the test explicitly specifying exactly what it depends > on than predicting changes. > >> I would consider if anybody >> remove this dependency without fixing test suite as bug which should be >> fixed. Also tests should detect this and report issues to stop this >> before patch like this is applied. > > True, but people do forget such things. I have sent a patch. Feel free to fix commit message if needed. Thanks, Michal ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] test/py: Fix MMC/SD block write test dependency
On 8/15/19 12:26 AM, Michal Simek wrote: On 14. 08. 19 17:51, Stephen Warren wrote: On 8/14/19 12:12 AM, Michal Simek wrote: On 13. 08. 19 23:21, Stephen Warren wrote: On 8/1/19 10:48 PM, Michal Simek wrote: Test is using random command which has own Kconfig symbol CMD_RANDOM which already depends on CMD_MEMORY. That's why replace cmd_memory by cmd_random. This might not always be true; I think it'd be better to keep the existing dependency list entries and just add CMD_RANDOM: Can you please tell me when exactly? I didn't see that. If in the future somebody changes the Kconfig file so that CMD_RANDOM doesn't depend on or select CMD_MEMORY. It may well not be possible right now; I just want to make sure the test doesn't rely on implementation details of CMD_RANDOM/CMD_MEMORY; if it explicitly needs both, it should simply depend on both. Do you think that we have used this rule in past? I hope so; I'd consider it a bug in a test if the test used a command but didn't explicitly depend on that command's CONFIG_ flag. We have had such bugs in the past, and fixed one or two of them. Anyway I understand what you mean but it is questionable if make sense to predict all changes which can happen. It's more about the test explicitly specifying exactly what it depends on than predicting changes. I would consider if anybody remove this dependency without fixing test suite as bug which should be fixed. Also tests should detect this and report issues to stop this before patch like this is applied. True, but people do forget such things. ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] test/py: Fix MMC/SD block write test dependency
On 14. 08. 19 17:51, Stephen Warren wrote: > On 8/14/19 12:12 AM, Michal Simek wrote: >> On 13. 08. 19 23:21, Stephen Warren wrote: >>> On 8/1/19 10:48 PM, Michal Simek wrote: Test is using random command which has own Kconfig symbol CMD_RANDOM which already depends on CMD_MEMORY. That's why replace cmd_memory by cmd_random. >>> >>> This might not always be true; I think it'd be better to keep the >>> existing dependency list entries and just add CMD_RANDOM: >> >> Can you please tell me when exactly? I didn't see that. > > If in the future somebody changes the Kconfig file so that CMD_RANDOM > doesn't depend on or select CMD_MEMORY. It may well not be possible > right now; I just want to make sure the test doesn't rely on > implementation details of CMD_RANDOM/CMD_MEMORY; if it explicitly needs > both, it should simply depend on both. Do you think that we have used this rule in past? Anyway I understand what you mean but it is questionable if make sense to predict all changes which can happen. I would consider if anybody remove this dependency without fixing test suite as bug which should be fixed. Also tests should detect this and report issues to stop this before patch like this is applied. Thanks, Michal ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] test/py: Fix MMC/SD block write test dependency
On 8/14/19 12:12 AM, Michal Simek wrote: On 13. 08. 19 23:21, Stephen Warren wrote: On 8/1/19 10:48 PM, Michal Simek wrote: Test is using random command which has own Kconfig symbol CMD_RANDOM which already depends on CMD_MEMORY. That's why replace cmd_memory by cmd_random. This might not always be true; I think it'd be better to keep the existing dependency list entries and just add CMD_RANDOM: Can you please tell me when exactly? I didn't see that. If in the future somebody changes the Kconfig file so that CMD_RANDOM doesn't depend on or select CMD_MEMORY. It may well not be possible right now; I just want to make sure the test doesn't rely on implementation details of CMD_RANDOM/CMD_MEMORY; if it explicitly needs both, it should simply depend on both. ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] test/py: Fix MMC/SD block write test dependency
On 13. 08. 19 23:21, Stephen Warren wrote: > On 8/1/19 10:48 PM, Michal Simek wrote: >> Test is using random command which has own Kconfig symbol CMD_RANDOM >> which >> already depends on CMD_MEMORY. That's why replace cmd_memory by >> cmd_random. > > This might not always be true; I think it'd be better to keep the > existing dependency list entries and just add CMD_RANDOM: Can you please tell me when exactly? I didn't see that. Thanks, Michal ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] test/py: Fix MMC/SD block write test dependency
On 8/1/19 10:48 PM, Michal Simek wrote: Test is using random command which has own Kconfig symbol CMD_RANDOM which already depends on CMD_MEMORY. That's why replace cmd_memory by cmd_random. This might not always be true; I think it'd be better to keep the existing dependency list entries and just add CMD_RANDOM: -@pytest.mark.buildconfigspec('cmd_mmc','cmd_memory') +@pytest.mark.buildconfigspec('cmd_mmc','cmd_random') ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] test/py: Fix MMC/SD block write test dependency
> Subject: [PATCH] test/py: Fix MMC/SD block write test dependency > > Test is using random command which has own Kconfig symbol CMD_RANDOM > which already depends on CMD_MEMORY. That's why replace cmd_memory > by cmd_random. > > Fixes: 09da18deab8b ("test/py: add MMC/SD block write test") > Signed-off-by: Michal Simek > --- > > test/py/tests/test_mmc_wr.py | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/test/py/tests/test_mmc_wr.py b/test/py/tests/test_mmc_wr.py > index 601279a6a4a3..7678c3c9c181 100644 > --- a/test/py/tests/test_mmc_wr.py > +++ b/test/py/tests/test_mmc_wr.py > @@ -35,7 +35,7 @@ env__mmc_wr_configs = ( > > """ > > -@pytest.mark.buildconfigspec('cmd_mmc','cmd_memory') > +@pytest.mark.buildconfigspec('cmd_mmc','cmd_random') > def test_mmc_wr(u_boot_console, env__mmc_wr_config): > """Test the "mmc write" command. Applied to mmc/master. Thanks, Peng > > -- > 2.17.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] test/py: Fix MMC/SD block write test dependency
> Subject: [PATCH] test/py: Fix MMC/SD block write test dependency > > Test is using random command which has own Kconfig symbol CMD_RANDOM > which already depends on CMD_MEMORY. That's why replace cmd_memory > by cmd_random. > > Fixes: 09da18deab8b ("test/py: add MMC/SD block write test") > Signed-off-by: Michal Simek > --- > > test/py/tests/test_mmc_wr.py | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/test/py/tests/test_mmc_wr.py b/test/py/tests/test_mmc_wr.py > index 601279a6a4a3..7678c3c9c181 100644 > --- a/test/py/tests/test_mmc_wr.py > +++ b/test/py/tests/test_mmc_wr.py > @@ -35,7 +35,7 @@ env__mmc_wr_configs = ( > > """ > > -@pytest.mark.buildconfigspec('cmd_mmc','cmd_memory') > +@pytest.mark.buildconfigspec('cmd_mmc','cmd_random') > def test_mmc_wr(u_boot_console, env__mmc_wr_config): > """Test the "mmc write" command. Reviewed-by: Peng Fan > > -- > 2.17.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH] test/py: Fix MMC/SD block write test dependency
Test is using random command which has own Kconfig symbol CMD_RANDOM which already depends on CMD_MEMORY. That's why replace cmd_memory by cmd_random. Fixes: 09da18deab8b ("test/py: add MMC/SD block write test") Signed-off-by: Michal Simek --- test/py/tests/test_mmc_wr.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/py/tests/test_mmc_wr.py b/test/py/tests/test_mmc_wr.py index 601279a6a4a3..7678c3c9c181 100644 --- a/test/py/tests/test_mmc_wr.py +++ b/test/py/tests/test_mmc_wr.py @@ -35,7 +35,7 @@ env__mmc_wr_configs = ( """ -@pytest.mark.buildconfigspec('cmd_mmc','cmd_memory') +@pytest.mark.buildconfigspec('cmd_mmc','cmd_random') def test_mmc_wr(u_boot_console, env__mmc_wr_config): """Test the "mmc write" command. -- 2.17.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot