Project

General

Profile

action #112319

Better and earlier checks of test code against "wrong API usage"

Added by okurz 2 months ago. Updated about 2 months ago.

Status:
New
Priority:
Low
Assignee:
-
Category:
Feature requests
Target version:
Start date:
2022-06-13
Due date:
% Done:

0%

Estimated time:
Difficulty:

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 issues

Related to openQA Project - action #99663: Use more perl signatures - os-autoinst size:MWorkable2021-10-01

History

#1 Updated by okurz 2 months ago

  • Related to action #99663: Use more perl signatures - os-autoinst size:M added

#2 Updated by tinita 2 months ago

What do you mean with "earlier"?

#3 Updated by cdywan 2 months ago

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.

#4 Updated by tinita 2 months ago

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.

#5 Updated by tinita 2 months ago

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.

#6 Updated by okurz 2 months ago

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.

#7 Updated by cdywan 2 months ago

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

#8 Updated by tinita 2 months ago

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.

#9 Updated by cdywan 2 months ago

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".

#10 Updated by tinita 2 months ago

Like I suggested, i think it's easier we just talk in jitsi about how PPI could help us here.

#11 Updated by tinita 2 months ago

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

#12 Updated by okurz about 2 months ago

  • Target version changed from Ready to future

Also available in: Atom PDF