| Message ID | 20241115164514.2186763-1-jerome.forissier@linaro.org |
|---|---|
| State | Accepted |
| Commit | 20f641987f83c4679a1181d79a546a098f11f5ad |
| Headers | show |
| Series | [v2] test/cmd/wget.c: move net_test_wget() to the cmd test suite | expand |
On Fri, Nov 15, 2024 at 05:45:14PM +0100, Jerome Forissier wrote: > Since net_test_wget() is testing a command and is in test/cmd it should > be in the 'cmd' test suite, not 'lib'. > > Saving and restoring the values of the environment variables that the > test manipulates is necessary to avoid a regression when running the > whole ut test suite. A minimal reproducer is: > > $ ./u-boot -T -c "ut cmd net_test_wget; ut dm dm_test_eth_act" | \ > grep -E "(Test:|Failures:)" > > Reported-by: Simon Glass <sjg@chromium.org> > Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org> > --- > test/cmd/wget.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/test/cmd/wget.c b/test/cmd/wget.c > index fe26fee54c9..7570c065a10 100644 > --- a/test/cmd/wget.c > +++ b/test/cmd/wget.c > @@ -19,7 +19,7 @@ > #include <dm/test.h> > #include <dm/device-internal.h> > #include <dm/uclass-internal.h> > -#include <test/lib.h> > +#include <test/cmd.h> > #include <test/test.h> > #include <test/ut.h> > > @@ -206,6 +206,10 @@ static int sb_http_handler(struct udevice *dev, void *packet, > > static int net_test_wget(struct unit_test_state *uts) > { > + char *prev_ethact = env_get("ethact"); > + char *prev_ethrotate = env_get("ethrotate"); > + char *prev_loadaddr = env_get("loadaddr"); > + > sandbox_eth_set_tx_handler(0, sb_http_handler); > sandbox_eth_set_priv(0, uts); > > @@ -223,6 +227,10 @@ static int net_test_wget(struct unit_test_state *uts) > ut_assert_nextline("md5 for 00020000 ... 0002001f ==> 234af48e94b0085060249ecb5942ab57"); > ut_assert_console_end(); > > + env_set("ethact", prev_ethact); > + env_set("ethrotate", prev_ethrotate); > + env_set("loadaddr", prev_loadaddr); > + > return 0; > } > -LIB_TEST(net_test_wget, UTF_CONSOLE); > +CMD_TEST(net_test_wget, UTF_CONSOLE); I don't know why / how, but now this causes the "hash" unit test to fail, in both Azure and GitLab.
On 11/19/24 16:59, Tom Rini wrote: > On Fri, Nov 15, 2024 at 05:45:14PM +0100, Jerome Forissier wrote: > >> Since net_test_wget() is testing a command and is in test/cmd it should >> be in the 'cmd' test suite, not 'lib'. >> >> Saving and restoring the values of the environment variables that the >> test manipulates is necessary to avoid a regression when running the >> whole ut test suite. A minimal reproducer is: >> >> $ ./u-boot -T -c "ut cmd net_test_wget; ut dm dm_test_eth_act" | \ >> grep -E "(Test:|Failures:)" >> >> Reported-by: Simon Glass <sjg@chromium.org> >> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org> >> --- >> test/cmd/wget.c | 12 ++++++++++-- >> 1 file changed, 10 insertions(+), 2 deletions(-) >> >> diff --git a/test/cmd/wget.c b/test/cmd/wget.c >> index fe26fee54c9..7570c065a10 100644 >> --- a/test/cmd/wget.c >> +++ b/test/cmd/wget.c >> @@ -19,7 +19,7 @@ >> #include <dm/test.h> >> #include <dm/device-internal.h> >> #include <dm/uclass-internal.h> >> -#include <test/lib.h> >> +#include <test/cmd.h> >> #include <test/test.h> >> #include <test/ut.h> >> >> @@ -206,6 +206,10 @@ static int sb_http_handler(struct udevice *dev, void *packet, >> >> static int net_test_wget(struct unit_test_state *uts) >> { >> + char *prev_ethact = env_get("ethact"); >> + char *prev_ethrotate = env_get("ethrotate"); >> + char *prev_loadaddr = env_get("loadaddr"); >> + >> sandbox_eth_set_tx_handler(0, sb_http_handler); >> sandbox_eth_set_priv(0, uts); >> >> @@ -223,6 +227,10 @@ static int net_test_wget(struct unit_test_state *uts) >> ut_assert_nextline("md5 for 00020000 ... 0002001f ==> 234af48e94b0085060249ecb5942ab57"); >> ut_assert_console_end(); >> >> + env_set("ethact", prev_ethact); >> + env_set("ethrotate", prev_ethrotate); >> + env_set("loadaddr", prev_loadaddr); >> + >> return 0; >> } >> -LIB_TEST(net_test_wget, UTF_CONSOLE); >> +CMD_TEST(net_test_wget, UTF_CONSOLE); > > I don't know why / how, but now this causes the "hash" unit test to > fail, in both Azure and GitLab. CI passed on https://github.com/u-boot/u-boot/pull/700. How do I reproduce the problem? Thanks,
On Thu, Nov 28, 2024 at 04:37:13PM +0100, Jerome Forissier wrote: > > > On 11/19/24 16:59, Tom Rini wrote: > > On Fri, Nov 15, 2024 at 05:45:14PM +0100, Jerome Forissier wrote: > > > >> Since net_test_wget() is testing a command and is in test/cmd it should > >> be in the 'cmd' test suite, not 'lib'. > >> > >> Saving and restoring the values of the environment variables that the > >> test manipulates is necessary to avoid a regression when running the > >> whole ut test suite. A minimal reproducer is: > >> > >> $ ./u-boot -T -c "ut cmd net_test_wget; ut dm dm_test_eth_act" | \ > >> grep -E "(Test:|Failures:)" > >> > >> Reported-by: Simon Glass <sjg@chromium.org> > >> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org> > >> --- > >> test/cmd/wget.c | 12 ++++++++++-- > >> 1 file changed, 10 insertions(+), 2 deletions(-) > >> > >> diff --git a/test/cmd/wget.c b/test/cmd/wget.c > >> index fe26fee54c9..7570c065a10 100644 > >> --- a/test/cmd/wget.c > >> +++ b/test/cmd/wget.c > >> @@ -19,7 +19,7 @@ > >> #include <dm/test.h> > >> #include <dm/device-internal.h> > >> #include <dm/uclass-internal.h> > >> -#include <test/lib.h> > >> +#include <test/cmd.h> > >> #include <test/test.h> > >> #include <test/ut.h> > >> > >> @@ -206,6 +206,10 @@ static int sb_http_handler(struct udevice *dev, void *packet, > >> > >> static int net_test_wget(struct unit_test_state *uts) > >> { > >> + char *prev_ethact = env_get("ethact"); > >> + char *prev_ethrotate = env_get("ethrotate"); > >> + char *prev_loadaddr = env_get("loadaddr"); > >> + > >> sandbox_eth_set_tx_handler(0, sb_http_handler); > >> sandbox_eth_set_priv(0, uts); > >> > >> @@ -223,6 +227,10 @@ static int net_test_wget(struct unit_test_state *uts) > >> ut_assert_nextline("md5 for 00020000 ... 0002001f ==> 234af48e94b0085060249ecb5942ab57"); > >> ut_assert_console_end(); > >> > >> + env_set("ethact", prev_ethact); > >> + env_set("ethrotate", prev_ethrotate); > >> + env_set("loadaddr", prev_loadaddr); > >> + > >> return 0; > >> } > >> -LIB_TEST(net_test_wget, UTF_CONSOLE); > >> +CMD_TEST(net_test_wget, UTF_CONSOLE); > > > > I don't know why / how, but now this causes the "hash" unit test to > > fail, in both Azure and GitLab. > > CI passed on https://github.com/u-boot/u-boot/pull/700. How do I reproduce > the problem? Sigh. I think we managed to hit one of those bugs that disappears when we re-order objects slightly. This is also passing for me now, so I'll apply this later today.
On Fri, 15 Nov 2024 17:45:14 +0100, Jerome Forissier wrote: > Since net_test_wget() is testing a command and is in test/cmd it should > be in the 'cmd' test suite, not 'lib'. > > Saving and restoring the values of the environment variables that the > test manipulates is necessary to avoid a regression when running the > whole ut test suite. A minimal reproducer is: > > [...] Applied to u-boot/master, thanks!
diff --git a/test/cmd/wget.c b/test/cmd/wget.c index fe26fee54c9..7570c065a10 100644 --- a/test/cmd/wget.c +++ b/test/cmd/wget.c @@ -19,7 +19,7 @@ #include <dm/test.h> #include <dm/device-internal.h> #include <dm/uclass-internal.h> -#include <test/lib.h> +#include <test/cmd.h> #include <test/test.h> #include <test/ut.h> @@ -206,6 +206,10 @@ static int sb_http_handler(struct udevice *dev, void *packet, static int net_test_wget(struct unit_test_state *uts) { + char *prev_ethact = env_get("ethact"); + char *prev_ethrotate = env_get("ethrotate"); + char *prev_loadaddr = env_get("loadaddr"); + sandbox_eth_set_tx_handler(0, sb_http_handler); sandbox_eth_set_priv(0, uts); @@ -223,6 +227,10 @@ static int net_test_wget(struct unit_test_state *uts) ut_assert_nextline("md5 for 00020000 ... 0002001f ==> 234af48e94b0085060249ecb5942ab57"); ut_assert_console_end(); + env_set("ethact", prev_ethact); + env_set("ethrotate", prev_ethrotate); + env_set("loadaddr", prev_loadaddr); + return 0; } -LIB_TEST(net_test_wget, UTF_CONSOLE); +CMD_TEST(net_test_wget, UTF_CONSOLE);
Since net_test_wget() is testing a command and is in test/cmd it should be in the 'cmd' test suite, not 'lib'. Saving and restoring the values of the environment variables that the test manipulates is necessary to avoid a regression when running the whole ut test suite. A minimal reproducer is: $ ./u-boot -T -c "ut cmd net_test_wget; ut dm dm_test_eth_act" | \ grep -E "(Test:|Failures:)" Reported-by: Simon Glass <sjg@chromium.org> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org> --- test/cmd/wget.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)