QA » openQA Project

For instance, instead of calling record_serialresult from validate_script_output at all, why not just call record_resultfile directly, with more accurate output?

I think it's just a weird coincidence. I can't swear to it, but I think I've run into this before but not bothered to file an issue. The code doesn't look like it's changed recently.

Maybe it was done like that because the argument to validate_script_output() is a coderef and not a regex.

But it would be possible to convert the coderef to a string again with the B::Deparse module.
Additionally it would be nice if one could just pass a regex to validate_script_output(). I guess that's a very common usage.

Then displaying the regex would be even easier.

I just tested my idea with B::Deparse with the following function in console/sqlite3.pm:

validate_script_output("pwd", sub {
    m/foo/
});

This is the output:

# wait_serial expected: {
    package sqlite3;
    use warnings;
    use strict;
    /foo/;
}

# Result:
/root

Would that help?

But maybe it should also use its own function instead of record_serialresult() to avoid the misleading text.

mmm...it's a bit hard to understand, I think. As I said in comment #1 here, the implementation of record_serialresult isn't necessarily the problem here - arguably validate_script_output just shouldn't use it.

basically B::Deparse would turn your subroutine into a string again.

In your case it would look like this:

# wait_serial expected: {
    package your::package;
    use warnings;
    use strict;
    not(($_ =~ /^python3-kickstart-1-1.noarch$/));
}

And if we write our own function it could look like this:

# validate_script_output expected: {
    package your::package;
    use warnings;
    use strict;
    not(($_ =~ /^python3-kickstart-1-1.noarch$/));
}

yes, but I think your thinking is locked into this whole 'print what was expected' path because that's what record_serialresult does. But we don't actually need to do that. The output here doesn't have to be "expected X, got Y" at all. It can just be something like "validate_script_output on line X failed, output that did not validate was: Y" or something. We don't necessarily need to try and show right in the output what the validation code was, if it comes out awkward.

I mean...that still reads pretty strangely. also your check there looks a bit weird - is there really no $_ =~ or anything in it, or did it get removed by the parsing?

$_ =~ m/foo/

is really the same as

m/foo/

or

/foo/

because $_ is the default variable for regex matches, and the m in m/.../ is optional.

Maybe the result string should be on top, and this deparsed output of the the check subroutine comes below (with a short explanation), for people who would like to see it.

I just wrote my first openqa test a few weeks ago, and this wait_serial output was something which confused me too, and to me the check subroutine would be helpful additional output.

What do others think?

In any case, it should be easy to replace the current confusing output.

OK, yeah, that's really super-compressed and hard to understand, then. I think including the deparsed validation coderef can work, but the text around it needs to explain clearly what's going on.

I think we should try to abstract away anything that looks like perl. regex is fine but the idea is that the test API is basically its own language. This is also the reason why we have API methods like get_var and set_var.

that's a bit difficult in this case as validate_script_output explicitly compares against an arbitrary code ref, though :/

yes, true. How tinita did it in https://github.com/os-autoinst/os-autoinst/pull/1199 is actually just fine with stripping the automatically added use statements.

@tinita don't be shy and assign a ticket to yourself when you are working on one. When you can't because you are not part of a project yet in progress talk with your managers about missing steps in onboarding :) Added you now in the parent project https://progress.opensuse.org/projects/qa/settings/members

@tinita?

Thanks again for this, it's a big improvement :)