Hi Svyatoslav,
On mar., janv. 09, 2024 at 15:57, Svyatoslav <clamo...@gmail.com> wrote:
> 9 січня 2024 р. 15:54:32 GMT+02:00, Mattijs Korpershoek
> <mkorpersh...@baylibre.com> написав(-ла):
>>Hi Svyatoslav,
>>
>>Thank you for the patch.
>>
>>On ven., janv. 05, 2024 at 09:22, Svyatoslav Ryhel <clamo...@gmail.com> wrote:
>>
>>> From: Ion Agorria <i...@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 <i...@agorria.com>
>>> Signed-off-by: Svyatoslav Ryhel <clamo...@gmail.com>
>>
>>I confirm this fixes the problem reported in:
>>https://lore.kernel.org/all/87wmspm9e5....@baylibre.com/
>>
>>Tested-by: Mattijs Korpershoek <mkorpersh...@baylibre.com>
>>
>>The commit message is a bit long, but I don't mind it staying this way.
>>
>>Reviewed-by: Mattijs Korpershoek <mkorpersh...@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
