Re: [RFC PATCH v1 04/21] test: hush: Test hush variable expansion.

[Francis Laniel <francis.laniel@amarulasolutions.com>]

Hi.


First, sorry for the delayed answer and thank you a lot for all your reviews!

Le samedi 8 janvier 2022, 15:53:49 CET Simon Glass a écrit :
> Hi Francis,
> 
> On Fri, 31 Dec 2021 at 09:14, Francis Laniel
> 
> <francis.laniel@amarulasolutions.com> wrote:
> > This commit ensures shell variables are replaced by their values.
> > 
> > Signed-off-by: Francis Laniel <francis.laniel@amarulasolutions.com>
> > ---
> > 
> >  test/hush/Makefile |   1 +
> >  test/hush/dollar.c | 191 +++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 192 insertions(+)
> >  create mode 100644 test/hush/dollar.c
> > 
> > diff --git a/test/hush/Makefile b/test/hush/Makefile
> > index a3c9ae5106..feb4f71956 100644
> > --- a/test/hush/Makefile
> > +++ b/test/hush/Makefile
> > @@ -5,3 +5,4 @@
> > 
> >  obj-y += cmd_ut_hush.o
> >  obj-y += if.o
> > 
> > +obj-y += dollar.o
> > diff --git a/test/hush/dollar.c b/test/hush/dollar.c
> > new file mode 100644
> > index 0000000000..9313d145d6
> > --- /dev/null
> > +++ b/test/hush/dollar.c
> > @@ -0,0 +1,191 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * (C) Copyright 2021
> > + * Francis Laniel, Amarula Solutions, francis.laniel@amarulasolutions.com
> > + */
> > +
> > +#include <common.h>
> > +#include <command.h>
> > +#include <env_attr.h>
> > +#include <test/hush.h>
> > +#include <test/ut.h>
> > +
> > +static int hush_test_simple_dollar(struct unit_test_state *uts)
> > +{
> > +       console_record_reset_enable();
> > +       ut_assertok(run_command("echo $dollar_foo", 0));
> > +#pragma GCC diagnostic push
> > +#pragma GCC diagnostic ignored "-Wformat-zero-length"
> > +       /*
> > +        * Next line is empty as $dollar_foo was not set before, but
> > compiler +        * complains about the format being empty, so we disable
> > this warning +        * only for this line.
> > +        */
> > +       ut_assert_nextline("");
> > +#pragma GCC diagnostic pop
> 
> You might be able to dp:
> 
> ut_assert_nextline("%s", "");

Thank you for the trick! With this I was able to get rid of this bad warning 
canceling.

> 
> > +       ut_assert_console_end();
> > +
> > +       ut_assertok(run_command("echo ${dollar_foo}", 0));
> > +#pragma GCC diagnostic push
> > +#pragma GCC diagnostic ignored "-Wformat-zero-length"
> > +       /*
> > +        * Next line is empty as $dollar_foo was not set before, but
> > compiler +        * complains about the format being empty, so we disable
> > this warning +        * only for this line.
> > +        */
> > +       ut_assert_nextline("");
> > +#pragma GCC diagnostic pop
> > +       ut_assert_console_end();
> > +
> > +       ut_assertok(run_command("dollar_foo=bar", 0));
> > +
> > +       ut_assertok(run_command("echo $dollar_foo", 0));
> > +       ut_assert_nextline("bar");
> > +       ut_assert_console_end();
> > +
> > +       ut_assertok(run_command("echo ${dollar_foo}", 0));
> > +       ut_assert_nextline("bar");
> > +       ut_assert_console_end();
> > +
> > +       /*
> > +        * R is way of giving raw string literals in C++.
> > +        * It means a "\n" is a R string literal will not be interpreted
> > as line +        * feed but printed as "\n".
> > +        * GCC provides it for C as an extension.
> > +        */
> > +       ut_assertok(run_command(R"(dollar_foo=\$bar)", 0));
> > +
> > +       ut_assertok(run_command("echo $dollar_foo", 0));
> > +       ut_assert_nextline("$bar");
> > +       ut_assert_console_end();
> > +
> > +       ut_assertok(run_command("dollar_foo='$bar'", 0));
> > +
> > +       ut_assertok(run_command("echo $dollar_foo", 0));
> > +       ut_assert_nextline("$bar");
> > +       ut_assert_console_end();
> > +
> > +       ut_asserteq(1, run_command("dollar_foo=bar quux", 0));
> > +       /* Next line contains error message. */
> > +       ut_assert_skipline();
> > +       ut_assert_console_end();
> > +
> > +       ut_asserteq(1, run_command("dollar_foo='bar quux", 0));
> > +       /* Next line contains error message. */
> > +       ut_assert_skipline();
> > +       ut_assert_console_end();
> > +
> > +       ut_asserteq(1, run_command(R"(dollar_foo=bar quux")", 0));
> > +       /* Two next lines contain error message. */
> > +       ut_assert_skipline();
> > +       ut_assert_skipline();
> > +       ut_assert_console_end();
> > +
> > +       ut_assertok(run_command(R"(dollar_foo='bar "quux')", 0));
> > +
> > +       ut_assertok(run_command("echo $dollar_foo", 0));
> > +       /*
> > +        * This one is buggy.
> > +        * ut_assert_nextline(R"(bar "quux)");
> > +        * ut_assert_console_end();
> > +        *
> > +        * So, let's reset output:
> > +        */
> > +       console_record_reset_enable();
> > +
> > +       ut_asserteq(1, run_command(R"(dollar_foo="bar 'quux")", 0));
> > +       /* Next line contains error message. */
> > +       ut_assert_skipline();
> > +       ut_assert_console_end();
> > +
> > +       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();
> > +
> > +       puts("Beware: this test set local variable dollar_foo and it
> > cannot be unset!"); +
> > +       return 0;
> > +}
> > +HUSH_TEST(hush_test_simple_dollar, 0);
> > +
> > +static int hush_test_env_dollar(struct unit_test_state *uts)
> > +{
> > +       env_set("env_foo", "bar");
> > +       console_record_reset_enable();
> > +
> > +       ut_assertok(run_command("echo $env_foo", 0));
> > +       ut_assert_nextline("bar");
> > +       ut_assert_console_end();
> > +
> > +       ut_assertok(run_command("echo ${env_foo}", 0));
> > +       ut_assert_nextline("bar");
> > +       ut_assert_console_end();
> > +
> > +       /* Environment variables have precedence over local variable. */
> 
> spelling
> 
> also please avoid period before */
> 
> > +       ut_assertok(run_command("env_foo=quux", 0));
> > +       ut_assertok(run_command("echo ${env_foo}", 0));
> > +       ut_assert_nextline("bar");
> > +       ut_assert_console_end();
> > +
> > +       /* Clean up setting the variable. */
> > +       env_set("env_foo", NULL);
> > +
> > +       puts("Beware: this test set local variable env_foo and it cannot
> > be unset!");
> Don't you unset it in the previous line?

I unsetted the environment variable named env_foo, but the local variable 
sadly remained because it is not possible to unset local variable.
So, after running "ut hush", the following should occur:
=> ut hush
Running 20 hush tests
...
Failures: 0
=> echo env_foo
quux

> > +
> > +       return 0;
> > +}
> > +HUSH_TEST(hush_test_env_dollar, 0);
> > +
> > +static int hush_test_command_dollar(struct unit_test_state *uts)
> > +{
> > +       console_record_reset_enable();
> > +
> > +       ut_assertok(run_command(R"(dollar_bar="echo bar")", 0));
> > +
> > +       ut_assertok(run_command("$dollar_bar", 0));
> > +       ut_assert_nextline("bar");
> > +       ut_assert_console_end();
> > +
> > +       ut_assertok(run_command("${dollar_bar}", 0));
> > +       ut_assert_nextline("bar");
> > +       ut_assert_console_end();
> > +
> > +       ut_assertok(run_command(R"(dollar_bar="echo
> > +       bar")", 0));
> 
> newline in the middle of a string ?

I think this should be a problem with how the mailer rendered this because on 
my side it is:
ut_assertok(run_command(R"(dollar_bar="echo bar")", 0));
I will check this on v2 (which should arrive from within 15-30 minutes).

> 
> > +
> > +       ut_assertok(run_command("$dollar_bar", 0));
> > +       ut_assert_nextline("bar");
> > +       ut_assert_console_end();
> > +
> > +       ut_assertok(run_command(R"(dollar_bar='echo bar
> > +       ')", 0));
> > +
> > +       ut_assertok(run_command("$dollar_bar", 0));
> > +       ut_assert_nextline("bar");
> > +       ut_assert_console_end();
> > +
> > +       ut_assertok(run_command(R"(dollar_bar='echo bar\n')", 0));
> > +
> > +       ut_assertok(run_command("$dollar_bar", 0));
> > +       ut_assert_nextline("barn");
> > +       ut_assert_console_end();
> > +
> > +       ut_assertok(run_command("dollar_bar='echo $bar'", 0));
> > +
> > +       ut_assertok(run_command("$dollar_bar", 0));
> > +       ut_assert_nextline("$bar");
> > +       ut_assert_console_end();
> > +
> > +       ut_assertok(run_command("dollar_quux=quux", 0));
> > +       ut_assertok(run_command(R"(dollar_bar="echo $dollar_quux")", 0));
> > +
> > +       ut_assertok(run_command("$dollar_bar", 0));
> > +       ut_assert_nextline("quux");
> > +       ut_assert_console_end();
> > +
> > +       puts("Beware: this test sets local variable dollar_bar and
> > dollar_quux and they cannot be unset!"); +
> > +       return 0;
> > +}
> > +HUSH_TEST(hush_test_command_dollar, 0);
> > --
> > 2.25.1
> 
> Regards,
> Simon