istopwg / ippsample

IPP sample implementations.
http://istopwg.github.io/ippsample
Apache License 2.0
226 stars 83 forks source link

ipptool: Add directive to monitor "printer-state" or "printer-state-reasons" in the background during a test #221

Closed wifiprintguy closed 3 years ago

wifiprintguy commented 3 years ago

Recent issues reported against the IPP Everywhere Self Certification Test I-20 and I-20.1 where I-20 deadlocks because PWG Raster is handled as a streaming document format, which prevents I-20.1 from checking the "printer-state-reasons", causes a condition where the tests cannot complete successfully. What is needed is a way for the tests to be run in parallel. But we don't want to create a complex general purpose parallel testing facility - that could create a monster.

However, from discussions on the IPP WG reflector and in recent teleconferences, it was discussed that, if "ipptool" were able to monitor "printer-state-reasons" in a background thread while performing the test in the foreground thread, this would be useful for this purpose and for monitoring the completion of the action triggered by the operation.

Add to ipptool support for a new "MONITOR-PRINTER-STATE" directive like so:

MONITOR-PRINTER-STATE predicate [timeout]

where ipptool would perform a polling Get-Printer-Attributes operation looking for the matching Printer state and state reason conditions defined by "predicate", and "timeout" would specify a hard time limit after which the MONITOR-PRINTER-STATE would fail if the matching status wasn't achieved. (We can noodle on the design of this...)

Also define a MONITOR-JOB-STATE that could be used to monitor a Job state for completion:

MONITOR-JOB-STATE jobid predicate [timeout]

The logic would be basically the same but would poll the Job specified by jobid with Get-Job-Attributes until the matching conditions were achieved or the timeout occurred.

michaelrsweet commented 3 years ago

@wifiprintguy OK, so I don't think we want a separate timeout - there isn't a precedent for that in ipptool files. But we can still support REPEAT-LIMIT with the default being a hard stop when the main test is completed (i.e. the Print-Job contents have been received for monitoring for media-needed, for example).

That would make the syntax:

MONITOR-PRINTER printer-uri {
    test directives
}

MONITOR-JOB printer-uri job-id {
    test directives
}

For example, the following would be used to expect 'media-needed' in "printer-state-reasons" while printing a job:

MONITOR-PRINTER $uri {
    EXPECT printer-state-reason WITH-VALUE "/^media-needed/"
}

And this would wait for a job to complete:

MONITOR-JOB $uri $job-id {
    EXPECT job-state WITH-VALUE >6 REPEAT-LIMIT 1000
}

Thoughts?

wifiprintguy commented 3 years ago

I think that looks good!

For completeness, where would we guide users to put that directive in the test - could it be anywhere? Could / should it live at the start, before the STATUS directives, or or the NAME directive, or would it matter?

So this would probably be OK:

{
    NAME "Example Print-Job"
    OPERATION Print-Job
    GROUP operation-attributes-tag
    ATTR charset attributes-charset utf-8
    ATTR naturalLanguage attributes-natural-language en
    ATTR uri printer-uri $uri
    ATTR name requesting-user-name $user
    ATTR name job-name "color.jpg with print-color-mode=monochrome"
    ATTR mimeMediaType document-format "image/jpeg"
    GROUP job-attributes-tag
    ATTR keyword print-color-mode monochrome
    FILE "color.jpg"

    STATUS successful-ok
    STATUS server-error-busy REPEAT-MATCH REPEAT-LIMIT 30

    MONITOR-JOB $uri $job-id {
        EXPECT job-state WITH-VALUE >6 REPEAT-LIMIT 1000
    }

}

What about this?

{

    MONITOR-JOB $uri $job-id {
        EXPECT job-state WITH-VALUE >6 REPEAT-LIMIT 1000
    }

    NAME "Example Print-Job"
    OPERATION Print-Job
    GROUP operation-attributes-tag
    ATTR charset attributes-charset utf-8
    ATTR naturalLanguage attributes-natural-language en
    ATTR uri printer-uri $uri
    ATTR name requesting-user-name $user
    ATTR name job-name "color.jpg with print-color-mode=monochrome"
    ATTR mimeMediaType document-format "image/jpeg"
    GROUP job-attributes-tag
    ATTR keyword print-color-mode monochrome
    FILE "color.jpg"

    STATUS successful-ok
    STATUS server-error-busy REPEAT-MATCH REPEAT-LIMIT 30

}
wifiprintguy commented 3 years ago

Hmmm, also need to be clear about the semantics.

If the background MONITOR-JOB has its requirements satisfied before the foreground operation is complete (as would be the case for a fix for https://github.com/istopwg/ippeveselfcert/issues/68) then does that cause ipptool to abruptly terminate the foreground operation?

michaelrsweet commented 3 years ago

@wifiprintguy No, we'd define the semantics as both having completed successfully to pass the test.

michaelrsweet commented 3 years ago

@wifiprintguy I think the monitor should be able to appear anywhere in the test specification, and potentially you could monitor both a printer and a job at the same time (so two background threads).

One thing with job monitoring - it would require a Create-Job in a prior test with the monitoring happening for the Send-Document/URI operation. Or it can look for the job-name with Get-Jobs... Will think some more on this, but I might break out job monitoring as a separate task and only implement printer monitoring for our first pass as that is what ippeveselfcert requires...

wifiprintguy commented 3 years ago

@michaelrsweet, +1 to moving the job monitoring to be a separate feature. Let's stay focused on getting the printer state monitoring done ASAP. Timeline? Wondering if we need to do an interim patch (let ippeveselfcert tests I-20 / I-20.1 use JPEG if supported).

michaelrsweet commented 3 years ago

@wifiprintguy I'm actively working on the changes now, I expect they'll be done by the end of the week so I don't think we need the JPEG fallback at all...

michaelrsweet commented 3 years ago

OK, so this is the syntax I'm settling on since it satisfies what we need for ippeveselfcert and should be flexible for other situations:

MONITOR-PRINTER-STATE printer-uri {
    EXPECT attribute-name [ predicate(s) ]
    ...
}

where "attribute-name" can be "printer-state", "printer-state-reasons", or "printer-state-message".

Then in a subsequent test we can add PASS-IF-DEFINED, PASS-IF-NOT-DEFINED, FAIL-IF-DEFINED, and FAIL-IF-NOT-DEFINED directives to continue monitoring in a subsequent test, e.g.:

{
    NAME "Print and wait for media-empty"
    OPERATION Print-Job
    GROUP operation-attributes-tag
        ATTR ATTR charset attributes-charset utf-8
        ATTR language attributes-natural-language en
        ATTR uri printer-uri $uri
        ATTR name requesting-user-name $user
        ATTR mimeMediaType document-format $filetype
    GROUP job-attributes-tag
        ATTR integer copies 1
    FILE $filename

    MONITOR-PRINTER-STATE $uri {
        EXPECT printer-state-reasons WITH-VALUE "/^media-empty/" DEFINE-MATCH have_media_empty
    }

    STATUS successful-ok
    STATUS successful-ok-ignored-or-substituted-attributes
    STATUS server-error-job-canceled
    STATUS server-error-busy REPEAT-MATCH REPEAT-LIMIT 30
}

{
    NAME "Wait for media-empty"
    PASS-IF-DEFINED have_media_empty
    DELAY 5
    OPERATION Get-Printer-Attributes
    GROUP operation-attributes-tag
        ATTR ATTR charset attributes-charset utf-8
        ATTR language attributes-natural-language en
        ATTR uri printer-uri $uri
        ATTR name requesting-user-name $user
        ATTR keyword requested-attributes printer-state-reasons
    STATUS successful-ok
    EXPECT printer-state-reasons WITH-VALUE "/^media-needed/" REPEAT-NO-MATCH REPEAT-LIMIT 24
}
michaelrsweet commented 3 years ago

OK, I've pushed changes to the "ipptool" branch of OpenPrinting CUPS. Still needs some testing...

michaelrsweet commented 3 years ago

[master 9423d4b] Sync up with CUPS 2.4.0 changes.

wifiprintguy commented 3 years ago

Trying this out using a test file based on "pause-printer.test" and ippserver after doing:

$ git pull $ make clean && ./configure --prefix=/usr/local && make && make test $ sudo make install

The output from using the attached "pause-printer-with-monitor.test" looks like it doesn't seem to be working...is this correct? I based it off your example above.

smitty@Serenity ipptool % /usr/local/bin/ipptool -vt 'ipp://Serenity.local:8503/ipp/print/AirPrintPDF' pause-printer-with-monitor.test
"pause-printer-with-monitor.test":
    Pause-Printer:
        attributes-charset (charset) = utf-8
        attributes-natural-language (naturalLanguage) = en
        printer-uri (uri) = ipp://Serenity.local:8503/ipp/print/AirPrintPDF
    Pause-Printer with printer state monitoring                          [ipptool: Bad printer URI "$uri".
PASS]
        RECEIVED: 72 bytes in response
        status-code = successful-ok (successful-ok)
        attributes-charset (charset) = utf-8
        attributes-natural-language (naturalLanguage) = en
ipptool: Unexpected token PASS-IF-DEFINED seen on line 26 of "pause-printer-with-monitor.test".

[pause-printer-with-monitor.test.txt](https://github.com/istopwg/ippsample/files/6261489/pause-printer-with-monitor.test.txt)
michaelrsweet commented 3 years ago

Shoot, I probably forgot to implement PASS-IF-DEFINED (just tested that it ran the tests in the background...)

Please stand by whilst I fix that and finish up my test suite for this...

michaelrsweet commented 3 years ago

@wifiprintguy This is now fixed, try master and the included print-job-media-needed.test file in examples.

wifiprintguy commented 3 years ago

There is a timing issue when this fix to ipptool is added to ippeveselfcert - it seems that the background thread isn't started before the foreground thread performs its operation. I will continue testing, but it seems there is still some work to do.

michaelrsweet commented 3 years ago

I've tested this again, and the background thread starts immediately for me. I added debug messages in the do_monitor_printer_state function and confirmed that it is doing requests.

michaelrsweet commented 3 years ago

@wifiprintguy Do you still have issues with this?

wifiprintguy commented 3 years ago

Not currently - let's close it and if it comes back up, reopen.