istopwg / ippeveselfcert

IPP Everywhere Printer Self-Certification Tools
https://www.pwg.org/ipp/everywhere.html
Apache License 2.0
21 stars 6 forks source link

ipp-tests.test : I-16 Test - REPEAT-LIMIT of 30 is sometimes inadequate #20

Closed rodriere closed 7 years ago

rodriere commented 7 years ago

Test I-16 which attempts to Get-Job-Attributes until the Print-Job from I-12 is completed, uses a REPEAT-LIMIT of 30 on line 666:

EXPECT job-state OF-TYPE unknown|enum IN-GROUP job-attributes-tag COUNT 1 WITH-VALUE >6 REPEAT-NO-MATCH REPEAT-LIMIT 30

After the limit is reached, I-16 will not be repeated. This causes I-16 to pass even when the job from I-12 is in the processing state. In the case where I-16 (incorrectly?) passes with a processing job, if the job from I-12 is still not completed and no completed job exists, I-17 and I-18 can fail because they send a Get-Jobs request with which-jobs=completed and expect job attributes in the response. Similarly, in the case where I-16 passes with a processing job, if the job from I-12 is still not completed, the Cancel-Job operation from I-19 can also fail because it assumes the job from I-12 is completed and expects the returned status code of client-error-not-possible.

As Smith indicated for a different issue, polling is not desirable but is inevitable here. We found printers that could take about 60 iterations to pass but I advise a REPEAT-LIMIT of 100 so that we don't revisit this issue again for slower printer mechanisms.

michaelrsweet commented 7 years ago

OK, so this is another issue with ipptool - if the repeat limit is reached that is supposed to be a FAIL.

michaelrsweet commented 7 years ago

Fix committed to ippsample project... Will get mirrored here as soon as the CUPS 2.2.4 work is complete.

[master 5b43e67] ipptool did not error out when a repeated test failed https://github.com/istopwg/ippeveselfcert/issues/20

wifiprintguy commented 7 years ago

So I guess the source of the problem was a corner case with the handling of an out-of-range or unexpected value when handling "WITH-VALUE >6", because if I do something like the test below, the test fails as expected (FAIL and return value != 0) even before the fix:

    {
        DELAY 5

        NAME "Get printer-uuid using an IPP Get-Printer-Attributes operation"

        OPERATION Get-Printer-Attributes
        Version 2.0

        GROUP operation
        ATTR charset attributes-charset utf-8
        ATTR language attributes-natural-language en
        ATTR uri printer-uri $uri
        ATTR keyword requested-attributes printer-uuid

        STATUS successful-ok

        EXPECT printer-fake-attribute REPEAT-NO-MATCH REPEAT-LIMIT 5

    }
michaelrsweet commented 7 years ago

Yeah, the code was inconsistent about adding error messages to the current test when repeat was in use. Once I fixed that the test would consistently fail once the repeat limit was exceeded.

rodriere commented 7 years ago

While I am happy that reaching repeat limits will cause a failure with the new ipptool version, I wanted to remind this thread that a higher limit is necessary for slower printer mechanisms (6 ppm ISO color)

michaelrsweet commented 7 years ago

One of the other changes coming in ipptool is the ability to control the delay interval between repeats - previously it varied from 1 to 8 seconds (average about 6 seconds), so it was difficult to control the total amount of time provided for the test to succeed. Now we'll be able to specify a fixed interval along with the number of retries so that we can provide enough time, even for the slowest printers.

wifiprintguy commented 7 years ago

@rodriere what time limit do you suggest?

rodriere commented 7 years ago

@wifiprintguy We found printers that could take about 60 iterations (~ 2 minutes) to print the color.jpg and pass tests using the released ipptool version but I advised a REPEAT-LIMIT of 100 so that we don't revisit this issue again. However, if delays will be included within each repeat on the newer ipptool, then I suppose another fix could be to add a small delay while increasing the limit slightly.

michaelrsweet commented 7 years ago

I've merged the CUPS upstream changes into the ippeveselfcert repository, so all of the changes needed to support this change are now in place.