Bug #4494
closed![Author: Michael Kay](https://www.gravatar.com/avatar/db6526d63053f09b62e52c2da8b2230a?rating=PG&size=50&default=https%3A%2F%2Fassets.plan.io%2Fimages%2Fdefault_avatar.png)
![Assignee: Michael Kay](https://www.gravatar.com/avatar/db6526d63053f09b62e52c2da8b2230a?rating=PG&size=22&default=https%3A%2F%2Fassets.plan.io%2Fimages%2Fdefault_avatar.png)
Not testing for "early exit" condition
100%
Description
Some XSLT3 tests (for example sf-empty-001) are marked with <result early-exit-possible='true'>
. The test driver looks at this flag and puts a comment in the result file if the flag was present in the test metadata but the XML input file was read to completion. The result file contains many comments of the form "Failed to make early exit", but we have been ignoring these comments and treating the test as successful.
Updated by Michael Kay over 4 years ago
- Status changed from New to In Progress
When early exit occurs, e.g. in test sf-empty-001
, we are correctly throwing a QuitParsingException
. This is being reported as a warning to the test driver's ErrorCollector
, but this is failing to recognise it as a QuitParsingException
and is therefore failing to set the madeEarlyExit
flag.
Fixed this, and the sf-empty test set is now seeing four "early exit" failures - on tests 004, 100, 101, and 103.
It's possible, of course, that there are tests where someone decided early exit is possible but either (a) it isn't, or (b) Saxon isn't able to detect it. We'll probably need to find some way of marking these tests in the exceptions file. (Perhaps the exceptions file should move closer to being a kind of "diff" on the main test set file, implemented by doing a transformation on the test set file before we run any tests?)
Updated by Michael Kay over 4 years ago
Looking at one of the failing tests, exists-004, and it appears that it is doing an early exit, but failing to report the fact.
If we compare with exists-001,
-
in exists-001,
ExistsStreamer.action()
callsgetTerminator().terminate()
; this throws theQuitParsingException
, which is caught by inWatchManager.startElement()
, resulting in a call towarning()
. -
in exists-004,
ExistsStreamer.action()
callsgetTerminator().terminate()
; this throws theQuitParsingException
, which is caught and rethrown a couple of times byItemFeed.processItems()
, and is then caught beWatchManager.endElement()
. This issues the requisite number ofendElement()
calls before rethrowing the exception; there is code inWatchManager.endElement()
to issue the warning, but it is commented out. (This is also true in 9.9)
Reinstating the code to issue the warning makes the test pass (the early exit is now detected by the test driver).
(Perhaps the warning was removed because it was considered inappropriate to issue warnings when everything is running correctly and to plan? This should really be an "info" message rather than a warning. Perhaps the new ErrorReporter
interface gives us an opportunity to add another severity level to the condition being reported?)
We're now getting test "failures" on a number of tests such as sf-exists-104 to -109 because they're doing an early exit when the test doesn't suggest it. This clearly shouldn't be a test failure, though it's an opportunity for improving the test. I've changed the test in these cases to expect early exit. That's still leaving failures in -100 .. -104 where early exit is expected by isn't reported as happening.
Looking at exists-100, the QuitParsingException
is being thrown as before, but this time during execution of WatchManager.deferredStartDocument()
called from WatchManager.startElement()
, and on this path the QuitParsingException
isn't being caught and notified. Fixed this by moving the call on WatchManager.deferredStartDocument()
within the scope of the try/catch.
This leaves exists-101. This time the QuitParsingException
is thrown during the call on endDocument
. It's not clear that we should really count that as being 'early exit'. What's happening in this test is that the argument to exists contains first, nodes from the streamed document, and then, a sequence of atomic values; and it's the sequence of atomic values that isn't being read to completion. It's a bit arbitrary what we report in this case so long as the test actually succeeds, so I shall simply change the test to remove the "early exit" label.
Updated by Michael Kay over 4 years ago
Looking at other test sets, some of the "early-exit-possible" tags seem questionable. For example sx-intersect-021 does
select="(/BOOKLIST/BOOKS/ITEM[1]//text() intersect $insertion
Saxon works out statically that the intersection is empty by type analysis; this means the streamed document is never read, so there is no early exit. So we can't afford to be too strict in evaluating these results.
Updated by Michael Kay over 4 years ago
- Status changed from In Progress to Resolved
- Fix Committed on Branch 10 added
I'm going to close this now.
We have made:
-
Changes to the test driver: added an option -strict that does stricter checking of the results
-
Changes to XSLT3 tests to change the early-exit-possible flag on some tests
-
Changes to WatchManager to ensure that the early-exit condition is reported to the ErrorReporter
Updated by O'Neil Delpratt over 4 years ago
- % Done changed from 0 to 100
- Fixed in Maintenance Release 10.1 added
Bug fix committed in the Saxon 10.1 maintenance release.
Updated by O'Neil Delpratt over 4 years ago
- Status changed from Resolved to Closed
Please register to edit this issue