Re: [PATCH v7 7/7] test: hush: dollar: fix bagous behavior

[Mattijs Korpershoek <mkorpershoek@baylibre.com>]

Hi Svyatoslav,

Thank you for the patch.

On ven., janv. 05, 2024 at 09:22, Svyatoslav Ryhel <clamor95@gmail.com> wrote:

> From: Ion Agorria <ion@agorria.com>
>
> The dollar test was merged with bagous console behavior, and
> instead of fixing it, this behavior was just workarounded.
>
> It seems like without the fix the ut_assert_skipline(); didn't clear
> console and running ut_assert_skipline(); many times would give always
> OK. With
>
> lib: membuff: fix readline not returning line in case of overflow
>
> the line is cleared correctly and next assert fails because now there
> is nothing to clean which is correct if we look the this a bit above
> the failing assert:
>
>     if (gd->flags & GD_FLG_HUSH_MODERN_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 if (gd->flags & GD_FLG_HUSH_OLD_PARSER) {
>          ut_assert_console_end();
>     }
>
> Which further confirms that tests workaround the old problem and now
> that problem is fixed we can remove the whole if blocks and simply
> place ut_assert_console_end() right after ut_assert_skipline() without
> any conditional and will pass green.
>
> So this part of code goes from:
>     ut_assert_skipline();
>     ut_assert_skipline();
>
>     if (gd->flags & GD_FLG_HUSH_MODERN_PARSER) {
>         /* See above comments. */
>         console_record_reset_enable();
>     } else if (gd->flags & GD_FLG_HUSH_OLD_PARSER) {
>         ut_assert_console_end();
>     }
>
> to become:
>     ut_assert_skipline();
>     ut_assert_console_end();
>
> Same thing should be done with the if block mentioned above that calls
> console_record_reset_enable().
>
> Signed-off-by: Ion Agorria <ion@agorria.com>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>

I confirm this fixes the problem reported in:
https://lore.kernel.org/all/87wmspm9e5.fsf@baylibre.com/

Tested-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>

The commit message is a bit long, but I don't mind it staying this way.

Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>

> ---
>  test/hush/dollar.c | 23 +++--------------------
>  1 file changed, 3 insertions(+), 20 deletions(-)
>
> diff --git a/test/hush/dollar.c b/test/hush/dollar.c
> index 4caa07c192..68d0874d90 100644
> --- a/test/hush/dollar.c
> +++ b/test/hush/dollar.c
> @@ -53,29 +53,12 @@ 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 (gd->flags & GD_FLG_HUSH_MODERN_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 if (gd->flags & GD_FLG_HUSH_OLD_PARSER) {
> -		ut_assert_console_end();
> -	}
> +	ut_assert_console_end();
>  
>  	ut_asserteq(1, run_command("dollar_foo=bar quux\"", 0));
> -	/* Two next lines contain error message */
> -	ut_assert_skipline();
> +	/* Next line contains error message */
>  	ut_assert_skipline();
> -
> -	if (gd->flags & GD_FLG_HUSH_MODERN_PARSER) {
> -		/* See above comments. */
> -		console_record_reset_enable();
> -	} else if (gd->flags & GD_FLG_HUSH_OLD_PARSER) {
> -		ut_assert_console_end();
> -	}
> +	ut_assert_console_end();
>  
>  	ut_assertok(run_command("dollar_foo='bar \"quux'", 0));
>  
> -- 
> 2.40.1