action #112319
open
Better and earlier checks of test code against "wrong API usage"
Added by okurz over 2 years ago.
Updated over 2 years ago.
Category:
Feature requests
Description
Motivation¶
https://openqa.suse.de/tests/8945569#step/firefox_nss/36 failed with
# Test died: Odd name/value argument for subroutine 'testapi::assert_and_click' at sle/tests/fips/mozilla_nss/firefox_nss.pm line 173.
testapi::assert_and_click("firefox-enter-password-OK", 120) called at sle/tests/fips/mozilla_nss/firefox_nss.pm line 173
as the code was actually using wrong format with assert_and_click('foo', 42)
but we enforced the correct usage with https://github.com/os-autoinst/os-autoinst/pull/2078 with signatures. There is no doubt that the usage is wrong and likely the best approach if people ignore the warnings is to actually fail the tests with an error but why so late during the execution for the test for "obviously" wrong code? Can we find a better and especially earlier check of wrong test code or wrong API usage in test code?
- Related to action #99663: Use more perl signatures - os-autoinst size:M added
What do you mean with "earlier"?
tinita wrote:
What do you mean with "earlier"?
Since we were talking about it in the daily I assume it means that the code testapi::assert_and_click("firefox-enter-password-OK", 120)
can never work. The signature is wrong. But either parsing or static analysis should be able to catch that. In practice the code fails "late" same as without signatures.
In perl, you can create subroutines at runtime. So perl can only check the correct usage at runtime.
But there might be a way with https://metacpan.org/pod/PPI to do it.
Unfortunately subroutines don't have introspection.
Especially the odd number for hash assignments is impossible to check in all cases because there might be foo(@array_with_arguments)
and it would depend on the number of items in the array if the check for an odd number would fail.
The goal should be that one can more easily distinguish "obvious wrong API usage" test failures from "job fails due to test and product not behaving as expected". Like if we would end up with "incompletes" then it would be more clear.
How about evaluating the complete test code before we run it on a real backend? I assume we could use the "null" backend up to a certain point and try to "execute" the complete code. Certainly a challenge is that the test code can follow certain paths based on conditions which can behave different in real cases.
tinita wrote:
In perl, you can create subroutines at runtime. So perl can only check the correct usage at runtime.
But there might be a way with https://metacpan.org/pod/PPI to do it.
Unfortunately subroutines don't have introspection.
From a quick test this unfortunately seems of limited use to us. The parsed document becomes incomplete if a brace is missing. Other changes concerning arguments, imports or sub names make no difference. The error is never set 🤔️
perl -e 'use PPI;my $doc=PPI::Document->new("tests/x11/chromium.pm");print $doc->errstr . "\n";die "incomplete" unless $doc->complete
@cdywan I'm not sure what you expect from your code. you just parse a perl file and print out if it was parsed successfully or not.
tinita wrote:
@cdywan I'm not sure what you expect from your code. you just parse a perl file and print out if it was parsed successfully or not.
I'm also printing errstr. So I would expect it to contain an error message like "brace missing on line 15" or whatever reasoning was used to decide that the file is not considered "complete".
Like I suggested, i think it's easier we just talk in jitsi about how PPI could help us here.
Ok, trying to explain here:
PPI parses a perl source file.
It does not involve perl itself executing the file, or doing a syntax check.
Actually, only perl can parse Perl, but PPI is doing a reasonable good job, with limitations the docs explain.
So if we want to do a syntax check, that will be done anyway by perl when running it, and it will fail early. Additionally we can run perl -c file.pm
to only do the syntax check.
My suggestion to use PPI was about:
- Parse the source files with PPI (possibly after doing a syntax check)
- Parse testapi.pm with PPI
- Go through the nodes in the parse tree to find the ones which contain the subroutine headers
- See if we get enough information to be able to tell if the usage matches the corresponding signature in testapi.pm or not
- Target version changed from Ready to future
Also available in: Atom
PDF