action #54548
closedUse of `record_serialresult` in `validate_script_output` is very confusing
100%
Description
This dates back all the way to this commit in 2014:
https://github.com/os-autoinst/os-autoinst/commit/0f4d22fd8c0924b1b448f65084cd03b892787c00
it introduced validate_script_output
, with a bit that was commented at the time as being dodgy:
# abusing the function
$autotest::current_test->record_serialresult($output, $res);
well, it's still basically there and it's still dodgy. Currently it reads:
# abusing the function
$autotest::current_test->record_serialresult($output, $res, $output) unless ($args{quiet});
but this does the same thing. In practice what this means is, every time validate_script_output
is called, you get a text thumbnail in your test which reads:
# wait_serial expected: (whatever the output was)
# Result:
(whatever the output was)
The two strings will always be identical. If the validate_script_output
check failed, the thumbnail will be bordered red, indicating a failure.
In the Fedora tests, we have this check:
validate_script_output 'rpm -q python3-kickstart', sub { $_ !~ m/^python3-kickstart-1-1.noarch$/ };
which basically means "ensure the output of rpm -q python3-kickstart is anything but 'python3-kickstart-1-1.noarch'". When this fails you get an extremely confusing result in the UI: you get a red-bordered text thumbnail reading:
# wait_serial expected: python3-kickstart-1-1.noarch
# Result:
python3-kickstart-1-1.noarch
That does not make it obvious at all what the problem is, and is arguably more misleading than just not including the thumbnail at all.
Really, though, even if the check was a positive assertion, the thumbnail would be equally misleading, because of how both strings will always be the exact output of the command - despite the wording "expected: ", that first string is never actually any kind of representation of what the check "expected".
I'm not sure why this was ever thought to be a good idea, but perhaps we should rethink it?
Example of the above case: https://openqa.fedoraproject.org/tests/424602
Files