| Message ID | 20211231161327.24918-17-francis.laniel@amarulasolutions.com |
|---|---|
| State | RFC |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | Modernize U-Boot shell | expand |
Hi Francis, On Fri, 31 Dec 2021 at 09:14, Francis Laniel <francis.laniel@amarulasolutions.com> wrote: > > This commit modifies the expected result for hush 2021. > Indeed, there were bugs in actual U-Boot hush which were fixed in upstream > Busybox. > As hush 2021 is based on upstream Busybox, these bugs no longer exist. > > Signed-off-by: Francis Laniel <francis.laniel@amarulasolutions.com> > --- > test/hush/dollar.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 51 insertions(+) Reviewed-by: Simon Glass <sjg@chromium.org> Can you avoid the preprocessor macros? Also, for testing we really want to be able to run both sets of tests in sandbox, so the parser needs to be run-time selectable. Regards, Simon
Le mercredi 12 janvier 2022, 21:03:37 CET Simon Glass a écrit : > Hi Francis, > > On Fri, 31 Dec 2021 at 09:14, Francis Laniel > > <francis.laniel@amarulasolutions.com> wrote: > > This commit modifies the expected result for hush 2021. > > Indeed, there were bugs in actual U-Boot hush which were fixed in upstream > > Busybox. > > As hush 2021 is based on upstream Busybox, these bugs no longer exist. > > > > Signed-off-by: Francis Laniel <francis.laniel@amarulasolutions.com> > > --- > > > > test/hush/dollar.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 51 insertions(+) > > Reviewed-by: Simon Glass <sjg@chromium.org> > > Can you avoid the preprocessor macros? > > Also, for testing we really want to be able to run both sets of tests > in sandbox, so the parser needs to be run-time selectable. To be sure, you would like to have a global variable named, for example, parser which value could be: * parser = "old", for old hush parser * parser = "2021", for "new" hush parser Is my understanding correct? > Regards, > Simon
Hi Francis, On Sun, 6 Feb 2022 at 11:23, Francis Laniel <francis.laniel@amarulasolutions.com> wrote: > > Le mercredi 12 janvier 2022, 21:03:37 CET Simon Glass a écrit : > > Hi Francis, > > > > On Fri, 31 Dec 2021 at 09:14, Francis Laniel > > > > <francis.laniel@amarulasolutions.com> wrote: > > > This commit modifies the expected result for hush 2021. > > > Indeed, there were bugs in actual U-Boot hush which were fixed in upstream > > > Busybox. > > > As hush 2021 is based on upstream Busybox, these bugs no longer exist. > > > > > > Signed-off-by: Francis Laniel <francis.laniel@amarulasolutions.com> > > > --- > > > > > > test/hush/dollar.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++ > > > 1 file changed, 51 insertions(+) > > > > Reviewed-by: Simon Glass <sjg@chromium.org> > > > > Can you avoid the preprocessor macros? > > > > Also, for testing we really want to be able to run both sets of tests > > in sandbox, so the parser needs to be run-time selectable. > > To be sure, you would like to have a global variable named, for example, > parser which value could be: > * parser = "old", for old hush parser > * parser = "2021", for "new" hush parser > Is my understanding correct? Yes although an int would be better than a string, perhaps controlled by a flag in global_data and a 'cli' subcommand to change the setting. Regards, Simon
Hi. (Sorry for the very late reply) Le lundi 7 février 2022, 21:22:20 CET Simon Glass a écrit : > Hi Francis, > > On Sun, 6 Feb 2022 at 11:23, Francis Laniel > > <francis.laniel@amarulasolutions.com> wrote: > > Le mercredi 12 janvier 2022, 21:03:37 CET Simon Glass a écrit : > > > Hi Francis, > > > > > > On Fri, 31 Dec 2021 at 09:14, Francis Laniel > > > > > > <francis.laniel@amarulasolutions.com> wrote: > > > > This commit modifies the expected result for hush 2021. > > > > Indeed, there were bugs in actual U-Boot hush which were fixed in > > > > upstream > > > > Busybox. > > > > As hush 2021 is based on upstream Busybox, these bugs no longer exist. > > > > > > > > Signed-off-by: Francis Laniel <francis.laniel@amarulasolutions.com> > > > > --- > > > > > > > > test/hush/dollar.c | 51 > > > > ++++++++++++++++++++++++++++++++++++++++++++++ > > > > 1 file changed, 51 insertions(+) > > > > > > Reviewed-by: Simon Glass <sjg@chromium.org> > > > > > > Can you avoid the preprocessor macros? > > > > > > Also, for testing we really want to be able to run both sets of tests > > > in sandbox, so the parser needs to be run-time selectable. > > > > To be sure, you would like to have a global variable named, for example, > > parser which value could be: > > * parser = "old", for old hush parser > > * parser = "2021", for "new" hush parser > > Is my understanding correct? > > Yes although an int would be better than a string, perhaps controlled > by a flag in global_data and a 'cli' subcommand to change the > setting. If I understand correctly, that would mean to compile both the old and new parser? Hence, I think I will need to rename some functions in the new parser to avoid problems at this level. > Regards, > Simon
diff --git a/test/hush/dollar.c b/test/hush/dollar.c index 9313d145d6..df2ad043f4 100644 --- a/test/hush/dollar.c +++ b/test/hush/dollar.c @@ -73,13 +73,27 @@ static int hush_test_simple_dollar(struct unit_test_state *uts) ut_asserteq(1, run_command("dollar_foo='bar quux", 0)); /* Next line contains error message. */ ut_assert_skipline(); +#if CONFIG_IS_ENABLED(HUSH_2021_PARSER) + /* + * For some strange reasons, the console is not empty after running + * above command. + * So, we reset it to not have side effects for other tests. + */ + console_record_reset_enable(); +#else /* HUSH_OLD_PARSER */ ut_assert_console_end(); +#endif /* HUSH_OLD_PARSER */ ut_asserteq(1, run_command(R"(dollar_foo=bar quux")", 0)); /* Two next lines contain error message. */ ut_assert_skipline(); ut_assert_skipline(); +#if CONFIG_IS_ENABLED(HUSH_2021_PARSER) + /* See above comments. */ + console_record_reset_enable(); +#else /* HUSH_OLD_PARSER */ ut_assert_console_end(); +#endif /* HUSH_OLD_PARSER */ ut_assertok(run_command(R"(dollar_foo='bar "quux')", 0)); @@ -93,17 +107,35 @@ static int hush_test_simple_dollar(struct unit_test_state *uts) */ console_record_reset_enable(); +#if CONFIG_IS_ENABLED(HUSH_2021_PARSER) + /* + * Old parser returns an error because it waits for closing '\'', but + * this behavior is wrong as the '\'' is surrounded by '"', so no need + * to wait for a closing one. + */ + ut_assertok(run_command(R"(dollar_foo="bar 'quux")", 0)); + + ut_assertok(run_command("echo $dollar_foo", 0)); + ut_assert_nextline("bar 'quux"); + ut_assert_console_end(); +#else /* HUSH_OLD_PARSER */ ut_asserteq(1, run_command(R"(dollar_foo="bar 'quux")", 0)); /* Next line contains error message. */ ut_assert_skipline(); ut_assert_console_end(); +#endif /* HUSH_OLD_PARSER */ ut_assertok(run_command("dollar_foo='bar quux'", 0)); ut_assertok(run_command("echo $dollar_foo", 0)); ut_assert_nextline("bar quux"); ut_assert_console_end(); +#if CONFIG_IS_ENABLED(HUSH_2021_PARSER) + /* Reset local variable. */ + ut_assertok(run_command("dollar_foo=", 0)); +#else /* HUSH_OLD_PARSER */ puts("Beware: this test set local variable dollar_foo and it cannot be unset!"); +#endif /* HUSH_OLD_PARSER */ return 0; } @@ -131,7 +163,12 @@ static int hush_test_env_dollar(struct unit_test_state *uts) /* Clean up setting the variable. */ env_set("env_foo", NULL); +#if CONFIG_IS_ENABLED(HUSH_2021_PARSER) + /* Reset local variable. */ + ut_assertok(run_command("env_foo=", 0)); +#else /* HUSH_OLD_PARSER */ puts("Beware: this test set local variable env_foo and it cannot be unset!"); +#endif /* HUSH_OLD_PARSER */ return 0; } @@ -168,7 +205,15 @@ static int hush_test_command_dollar(struct unit_test_state *uts) ut_assertok(run_command(R"(dollar_bar='echo bar\n')", 0)); ut_assertok(run_command("$dollar_bar", 0)); +#if CONFIG_IS_ENABLED(HUSH_2021_PARSER) + /* + * This difference seems to come from a bug solved in Busybox hush. + * Behavior of hush 2021 is coherent with bash and other shells. + */ + ut_assert_nextline(R"(bar\n)"); +#else /* HUSH_OLD_PARSER */ ut_assert_nextline("barn"); +#endif /* HUSH_OLD_PARSER */ ut_assert_console_end(); ut_assertok(run_command("dollar_bar='echo $bar'", 0)); @@ -184,7 +229,13 @@ static int hush_test_command_dollar(struct unit_test_state *uts) ut_assert_nextline("quux"); ut_assert_console_end(); +#if CONFIG_IS_ENABLED(HUSH_2021_PARSER) + /* Reset local variables. */ + ut_assertok(run_command("dollar_bar=", 0)); + ut_assertok(run_command("dollar_quux=", 0)); +#else /* HUSH_OLD_PARSER */ puts("Beware: this test sets local variable dollar_bar and dollar_quux and they cannot be unset!"); +#endif /* HUSH_OLD_PARSER */ return 0; }
This commit modifies the expected result for hush 2021. Indeed, there were bugs in actual U-Boot hush which were fixed in upstream Busybox. As hush 2021 is based on upstream Busybox, these bugs no longer exist. Signed-off-by: Francis Laniel <francis.laniel@amarulasolutions.com> --- test/hush/dollar.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+)