https://progress.opensuse.org/https://progress.opensuse.org/themes/openSUSE/favicon/favicon.ico?15829177842019-07-23T01:21:19ZopenSUSE Project Management ToolopenQA Project - action #54548: Use of `record_serialresult` in `validate_script_output` is very confusinghttps://progress.opensuse.org/issues/54548?journal_id=2291662019-07-23T01:21:19ZAdamWill
<ul></ul><p>For instance, instead of calling <code>record_serialresult</code> from <code>validate_script_output</code> <em>at all</em>, why not just call <code>record_resultfile</code> directly, with more accurate output?</p>
openQA Project - action #54548: Use of `record_serialresult` in `validate_script_output` is very confusinghttps://progress.opensuse.org/issues/54548?journal_id=2309362019-07-30T07:54:04Zokurzokurz@suse.com
<ul><li><strong>Related to</strong> <i><a class="issue tracker-4 status-3 priority-4 priority-default closed" href="/issues/54347">action #54347</a>: [tools] Wrong message on serial missmatch</i> added</li></ul> openQA Project - action #54548: Use of `record_serialresult` in `validate_script_output` is very confusinghttps://progress.opensuse.org/issues/54548?journal_id=2309422019-07-30T07:55:12Zokurzokurz@suse.com
<ul><li><strong>Category</strong> set to <i>Regressions/Crashes</i></li></ul><p>I wonder if it really took 5 years for you <em>and</em> dheidler to report about the same issue but then within the <em>very same week</em> – see <a href="https://progress.opensuse.org/issues/54347" class="external">https://progress.opensuse.org/issues/54347</a> – or if we really have a regression now.</p>
openQA Project - action #54548: Use of `record_serialresult` in `validate_script_output` is very confusinghttps://progress.opensuse.org/issues/54548?journal_id=2312152019-07-30T14:45:23ZAdamWill
<ul></ul><p>I <em>think</em> 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.</p>
openQA Project - action #54548: Use of `record_serialresult` in `validate_script_output` is very confusinghttps://progress.opensuse.org/issues/54548?journal_id=2352262019-08-14T14:42:10Ztinitatina.mueller+trick-redmine@suse.com
<ul></ul><p>Maybe it was done like that because the argument to validate_script_output() is a coderef and not a regex.</p>
<p>But it would be possible to convert the coderef to a string again with the B::Deparse module.<br>
Additionally it would be nice if one could just pass a regex to validate_script_output(). I guess that's a very common usage.</p>
<p>Then displaying the regex would be even easier.</p>
openQA Project - action #54548: Use of `record_serialresult` in `validate_script_output` is very confusinghttps://progress.opensuse.org/issues/54548?journal_id=2352352019-08-14T15:07:12Ztinitatina.mueller+trick-redmine@suse.com
<ul></ul><p>I just tested my idea with B::Deparse with the following function in console/sqlite3.pm:</p>
<pre><code>validate_script_output("pwd", sub {
m/foo/
});
</code></pre>
<p>This is the output:</p>
<pre><code># wait_serial expected: {
package sqlite3;
use warnings;
use strict;
/foo/;
}
# Result:
/root
</code></pre>
<p>Would that help?</p>
<p>But maybe it should also use its own function instead of record_serialresult() to avoid the misleading text.</p>
openQA Project - action #54548: Use of `record_serialresult` in `validate_script_output` is very confusinghttps://progress.opensuse.org/issues/54548?journal_id=2352382019-08-14T15:13:03ZAdamWill
<ul></ul><p>mmm...it's a bit hard to understand, I think. As I said in comment #1 here, the implementation of <code>record_serialresult</code> isn't necessarily the problem here - arguably <code>validate_script_output</code> just shouldn't use it.</p>
openQA Project - action #54548: Use of `record_serialresult` in `validate_script_output` is very confusinghttps://progress.opensuse.org/issues/54548?journal_id=2352412019-08-14T15:19:18Ztinitatina.mueller+trick-redmine@suse.com
<ul></ul><p>basically B::Deparse would turn your subroutine into a string again.</p>
<p>In your case it would look like this:</p>
<pre><code># wait_serial expected: {
package your::package;
use warnings;
use strict;
not(($_ =~ /^python3-kickstart-1-1.noarch$/));
}
</code></pre>
<p>And if we write our own function it could look like this:</p>
<pre><code># validate_script_output expected: {
package your::package;
use warnings;
use strict;
not(($_ =~ /^python3-kickstart-1-1.noarch$/));
}
</code></pre> openQA Project - action #54548: Use of `record_serialresult` in `validate_script_output` is very confusinghttps://progress.opensuse.org/issues/54548?journal_id=2352442019-08-14T15:27:13ZAdamWill
<ul></ul><p>yes, but I think your thinking is locked into this whole 'print what was expected' path because that's what <code>record_serialresult</code> does. But <em>we don't actually need to do that</em>. 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.</p>
openQA Project - action #54548: Use of `record_serialresult` in `validate_script_output` is very confusinghttps://progress.opensuse.org/issues/54548?journal_id=2352502019-08-14T15:42:07Ztinitatina.mueller+trick-redmine@suse.com
<ul><li><strong>File</strong> <a href="/attachments/8426">validate-script-output.png</a> <a class="icon-only icon-download" title="Download" href="/attachments/download/8426/validate-script-output.png">validate-script-output.png</a> added</li></ul><p>But it would be nice to show what was expected anyway.</p>
<p>I attached a screenshot of what I just tried out.</p>
openQA Project - action #54548: Use of `record_serialresult` in `validate_script_output` is very confusinghttps://progress.opensuse.org/issues/54548?journal_id=2352532019-08-14T15:53:24ZAdamWill
<ul></ul><p>I mean...that still reads pretty strangely. also your check there looks a bit weird - is there really no <code>$_ =~</code> or anything in it, or did it get removed by the parsing?</p>
openQA Project - action #54548: Use of `record_serialresult` in `validate_script_output` is very confusinghttps://progress.opensuse.org/issues/54548?journal_id=2352562019-08-14T16:00:07Ztinitatina.mueller+trick-redmine@suse.com
<ul></ul><pre><code>$_ =~ m/foo/
</code></pre>
<p>is really the same as</p>
<pre><code>m/foo/
</code></pre>
<p>or</p>
<pre><code>/foo/
</code></pre>
<p>because $_ is the default variable for regex matches, and the m in m/.../ is optional.</p>
<p>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.</p>
<p>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.</p>
<p>What do others think?</p>
<p>In any case, it should be easy to replace the current confusing output.</p>
openQA Project - action #54548: Use of `record_serialresult` in `validate_script_output` is very confusinghttps://progress.opensuse.org/issues/54548?journal_id=2352592019-08-14T16:09:31ZAdamWill
<ul></ul><p>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.</p>
openQA Project - action #54548: Use of `record_serialresult` in `validate_script_output` is very confusinghttps://progress.opensuse.org/issues/54548?journal_id=2352652019-08-14T18:26:10Zokurzokurz@suse.com
<ul></ul><p>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 <code>get_var</code> and <code>set_var</code>.</p>
openQA Project - action #54548: Use of `record_serialresult` in `validate_script_output` is very confusinghttps://progress.opensuse.org/issues/54548?journal_id=2352772019-08-14T18:45:00ZAdamWill
<ul></ul><p>that's a bit difficult in this case as <code>validate_script_output</code> <em>explicitly</em> compares against an arbitrary code ref, though :/</p>
openQA Project - action #54548: Use of `record_serialresult` in `validate_script_output` is very confusinghttps://progress.opensuse.org/issues/54548?journal_id=2353162019-08-15T04:53:21Zokurzokurz@suse.com
<ul></ul><p>yes, true. How tinita did it in <a href="https://github.com/os-autoinst/os-autoinst/pull/1199" class="external">https://github.com/os-autoinst/os-autoinst/pull/1199</a> is actually just fine with stripping the automatically added use statements.</p>
<p><a class="user active user-mention" href="https://progress.opensuse.org/users/33482">@tinita</a> 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 <a href="https://progress.opensuse.org/projects/qa/settings/members" class="external">https://progress.opensuse.org/projects/qa/settings/members</a></p>
openQA Project - action #54548: Use of `record_serialresult` in `validate_script_output` is very confusinghttps://progress.opensuse.org/issues/54548?journal_id=2353192019-08-15T04:53:32Zokurzokurz@suse.com
<ul><li><strong>Assignee</strong> set to <i>tinita</i></li></ul> openQA Project - action #54548: Use of `record_serialresult` in `validate_script_output` is very confusinghttps://progress.opensuse.org/issues/54548?journal_id=2354932019-08-15T10:47:24Ztinitatina.mueller+trick-redmine@suse.com
<ul><li><strong>Status</strong> changed from <i>New</i> to <i>In Progress</i></li><li><strong>% Done</strong> changed from <i>0</i> to <i>50</i></li><li><strong>Difficulty</strong> set to <i>easy</i></li></ul> openQA Project - action #54548: Use of `record_serialresult` in `validate_script_output` is very confusinghttps://progress.opensuse.org/issues/54548?journal_id=2409202019-09-03T11:21:12Zcoolocoolo@suse.com
<ul><li><strong>Target version</strong> set to <i>Current Sprint</i></li></ul> openQA Project - action #54548: Use of `record_serialresult` in `validate_script_output` is very confusinghttps://progress.opensuse.org/issues/54548?journal_id=2454352019-09-23T20:15:39Zokurzokurz@suse.com
<ul></ul><p><a class="user active user-mention" href="https://progress.opensuse.org/users/33482">@tinita</a>?</p>
openQA Project - action #54548: Use of `record_serialresult` in `validate_script_output` is very confusinghttps://progress.opensuse.org/issues/54548?journal_id=2464432019-09-26T11:35:56Ztinitatina.mueller+trick-redmine@suse.com
<ul><li><strong>Status</strong> changed from <i>In Progress</i> to <i>Feedback</i></li><li><strong>% Done</strong> changed from <i>50</i> to <i>100</i></li></ul><p>PR <a href="https://github.com/os-autoinst/os-autoinst/pull/1199" class="external">https://github.com/os-autoinst/os-autoinst/pull/1199</a> was merged.</p>
openQA Project - action #54548: Use of `record_serialresult` in `validate_script_output` is very confusinghttps://progress.opensuse.org/issues/54548?journal_id=2464522019-09-26T12:37:56Ztinitatina.mueller+trick-redmine@suse.com
<ul><li><strong>Status</strong> changed from <i>Feedback</i> to <i>Resolved</i></li></ul><p>Checked <a href="https://openqa.suse.de/tests/3409790#step/curl_https/4" class="external">https://openqa.suse.de/tests/3409790#step/curl_https/4</a></p>
openQA Project - action #54548: Use of `record_serialresult` in `validate_script_output` is very confusinghttps://progress.opensuse.org/issues/54548?journal_id=2465062019-09-26T15:05:06ZAdamWill
<ul></ul><p>Thanks again for this, it's a big improvement :)</p>