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

[Mattijs Korpershoek <mkorpershoek@baylibre.com>]

Hi Svyatoslav,

On mar., janv. 09, 2024 at 15:57, Svyatoslav <clamor95@gmail.com> wrote:

> 9 січня 2024 р. 15:54:32 GMT+02:00, 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>
>>
>
> Thanks! Commit title might have a typo. Not "bagous" but "bugous" should be correct. May you adjust on pick if not too hard?

Yes, will update while picking up.

>
>>> ---
>>>  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