Issue with Global Variables in coverage report in 3_0_3.

[birdya22]

I should say at the beginning that I've done my testing with the changes from #1917.

The stylesheet has the following content:
There are 7 global variables at the start with a variety of contents: xsl:text, xsl:for-each, xsl:choose, xsl:analyze-string. None of them are referenced within the rest of the stylesheet.
There are then 2 named templates:
TestTemplate01 outputs 100 in a root node.
TestTemplate02 outputs 200 in a root node.

CoverageGlobalVariableIssue01.xspec invokes TestTemplate01.
CoverageGlobalVariableIssue02.xspec invokes TestTemplate02.

CoverageGlobalVariableIssue01.xspec coverage report shows some parts of the global variables being hit and some missed, rather than all ignored.

CoverageGlobalVariableIssue02.xspec coverage report looks fine.

Adrian
GlobalVariableCoverageTest.zip

[galtm]

Adrian, thanks for the examples. When working on #1918, I looked out for whether the changes fixed your specific problem but I also wanted to know: Do the changes introduce side effects, especially code being marked as hit, when it's really missed? I generated coverage reports for files in this repo's test suite and noticed that my code changes affected the hit/missed/ignored status of some global XSLT parameters (specifically https://github.com/xspec/xspec/blob/master/test/issue-1564.xspec and https://github.com/xspec/xspec/blob/master/test/global-override.xspec if you or anyone else reading this is interested).

Looking into what the correct behavior should be is on my to-do list. Your examples will probably also be useful for determining whether #1918 is a correct fix or whether it needs adjustments.

[birdya22]

Amanda,
this is rather a long reply after some more investigation but the headline comment is that I think your change is good but some of the rules in the element() | text() template are probably causing problems, and this may be related to any changes that happened to the Saxon trace mechanism in 12.4 compared to when it previously worked (as there is no documentation about what it actually produces it must be difficult).
I haven't managed to get to grips with the references you provided.

Here are my thoughts. This is all from the point of only using Saxon-PE so there are a lot of features that I have never used, and cannot use.

I think the starting point should be a hit for everything that Saxon says is a hit, otherwise things get very tricky if you decide to disagree with Saxon and ignore some hits.
So I don't see anything wrong with your change because it more consistently captures hits in the coverage.xml file which should be the starting point.

So that leaves you with working out what to mark as a hit when it's not in the trace output, and when it's not a hit is it a miss or ignored.

From looking outside at this I would have a couple of questions (which I can partially answer from the coverage-report.xsl but I can't tell the rationale behind the rules in some cases):

1. Do you have a list of elements that are not in the trace output even when they are hit (and in what circumstances this happens)?
2. Do you have a written set of criteria that are used to define the situations when a node is hit/miss/ignored if it is not marked as hit in coverage.xml?
   Are the criteria tied down to the minimum use case?
   Do the criteria overlap in any ways such that there is a defined order in which they need to be applied?

From a bit more testing I have the following comments on my points 1 and 2 above (starting with some obvious ones):

1. Not Traced
   xsl:matching-substring
   xsl:non-matching-substring
   xsl:otherwise
   xsl:when
   xsl:variable - when it is global and uses @select for a string e.g: <xsl:variable name="var02" select="'4'" />
   xsl:param - in a xsl:function (even if it is used it is not in the trace).
   xsl:with-param - when it uses @select for a string <xsl:with-param name="param02" select="'800'" />
   (Not xsl:for-each or xsl:for-each-group as these are in the trace)

2. Criteria for deciding if hit/miss/ignored
   I don't see any issues with the template for xsl:when etc. except it doesn't need xsl:for-each and xsl:for-each-group.
   A couple of comments on the xsl:choose in the element() | text() template.
   In one of my tests I have a global xsl:param that is not used and it was marked as ignored. The rule which causes this is:
   <xsl:when test="empty(ancestor::xsl:*[parent::xsl:stylesheet or parent::xsl:transform])">ignored</xsl:when>
   I added in a rule above it and it changed to missed as you would expect:
   <xsl:when test="self::xsl:param and ((parent::xsl:stylesheet) or (parent::xsl:transform))">missed</xsl:when>

This may also be the reason why global variables have problems for me. I removed the 2 variable tests because I still think they shouldn't be needed as most xsl:variable elements are hit if used (but see below):
<xsl:when test="self::xsl:variable">
<xsl:sequence select="local:coverage(following-sibling::*[not(self::xsl:variable)][1], $module-id)" />
</xsl:when>
<xsl:when test="ancestor::xsl:variable">
<xsl:sequence select="local:coverage(ancestor::xsl:variable[1], $module-id)" />
</xsl:when>
By itself it makes parts of the xsl:variable ignored and parts missed. So I added in a global variable specific rule, like the xsl:param one:
<xsl:when test="self::xsl:variable and ((parent::xsl:stylesheet) or (parent::xsl:transform))">missed</xsl:when>
This is on the basis that if it isn't hit then it must be missed.

The comment for this rule
<xsl:when test="empty(ancestor::xsl:*[parent::xsl:stylesheet or parent::xsl:transform])">ignored</xsl:when>
says
but is the test having unintended consequences, like my global param being ignored.

I still see a problem for a global xsl:variable which has a select attribute with a string value because they are not hit in the trace when used. So I would say it should be marked as missed, but this is wrong and I don't know how you can get around this without getting Saxonica to change and include it in the trace.

Finally, my suggestion would be a review of the rules in xsl:choose in the element(0 | text() template to see if they are still doing that they are intended to do, whether they do more than was intended and whether any more are needed. I do realise the potential ramifications of this suggestion.

Note: what I found useful in the testing I did was something like a version of the coverage-report that only reports the hits from the coverage.xml file (this made it easy to see what was and wasn't being traced which is how I got some of the suggestions above). I had to knock something up to do this but I assume you have something similar.

Adrian

[birdya22]

I've had a look at the tests in both the links and the reason for the change is the same in both.
In both cases xsl:param elements were ignored but are now hit.
Basically in 3_0_3 there was a hit on the line containing the param element but because it appears as a variable and not a param in the uqname, uqname="Q{http://www.w3.org/1999/XSL/Transform}variable", 3_0_3 didn't record a hit. With your change you don't just rely on the uqname so you recognise it as a hit via the class, class="net.sf.saxon.expr.instruct.GlobalParam".
So I think this is a good thing.

With your change I think this raises a question about whether it is worth doing the comparison with the uqname at all because if it fails you will still record a hit if there is a class name, which seems to make the uqname check redundant. This is your change inside local:hits-on-node
<xsl:if test="($node-uqname eq $hit-traceable-uqname) or
exists($hit-traceable-class)">
There is probably a way to stick with the previous method of relying on the uqname check if this would work:
I believe my initial problem was the fact that the first hit with a class="net.sf.saxon.expr.instruct.TraceExpression" mapped to a choice node and so all other hits of that class also assumed the uqname was choice, which wasn't the case, and so my example missed some hits.
A possible alternative solution might be to check every time whether the class & associated uqname match and if not create a new traceable element to record the new match. That wouldn't require the change to coverage-report.xsl, but also it would mean that the param elements in the 2 tests would still be ignored rather than hit (assuming no other changes).

In 3_0_3 the param lines were shown as ignored which seems a bit strange to me as I would have expected to see them as missed if they weren't hit, particularly as the test xspec expects the param to provide the result value.
This is probably down to this test in the element() | text() template:
<xsl:when test="empty(ancestor::xsl:*[parent::xsl:stylesheet or parent::xsl:transform])">ignored</xsl:when>

Related to my previous comments, does this need re-visiting to think about which global elements should be ignored or missed if they are not hit (this will of course come down to what Saxon records hits on). The XSLT 3.0 spec lists the following declaration elements permitted in the xsl:stylesheet:
xsl:accumulator
xsl:attribute-set
xsl:character-map
xsl:decimal-format
xsl:function
xsl:global-context-item
xsl:import
xsl:import-schema
xsl:include
xsl:key
xsl:mode
xsl:namespace-alias
xsl:output
xsl:param
xsl:preserve-space
xsl:strip-space
xsl:template
xsl:use-package
xsl:variable

From your comment, "I generated coverage reports for files in this repo's test suite and noticed ..." does this mean that you have automated checking of coverage reports against your current baseline?

Here are the details for issue-1564 as it is the simpler of the 2.
In 3_0_3 line 6 was hit in the coverage.xml file but appears as a variable rather than a param and it was shown as ignored (I would have expected it to be shown as missed):
<traceable traceableId="1"
uqname="Q{http://www.w3.org/1999/XSL/Transform}variable"/>
<hit lineNumber="6" columnNumber="82" moduleId="0" traceableId="1"/>

With your changes you've added the @Class into the coverage.xml and that is used to make it a hit via the class and not the uqname. coverage.xml is now
<traceable traceableId="1"
class="net.sf.saxon.expr.instruct.GlobalParam"
uqname="Q{http://www.w3.org/1999/XSL/Transform}variable"/>
<hit lineNumber="6" columnNumber="82" moduleId="0" traceableId="1"/>
with this change inside local:hits-on-node
<xsl:if test="($node-uqname eq $hit-traceable-uqname) or
exists($hit-traceable-class)">

Adrian

[galtm]

Adrian, thanks a lot for your analysis and observations. I am hoping to have a block of uninterrupted time sometime in the next few days, to look into this and write back more substantively. Thanks for your patience!

[birdya22]

Amanda,
I noticed something new for you to add into the mix when considering what changes you want to make.
It is a problem in Saxon, a bit similar to my first issue. If there are xsl:if and xsl:choose elements encountered during tracing then whichever is encountered first, all xsl:if and xsl:choose elements are tagged with the first one encountered.

Saxon, and therefore xspec, only includes one of "if" or "choose" in the trace output even if both xsl:if and xsl:choose are present in the stylesheet.
I attached a little test case. 2 named templates with a xsl:if and xsl:choose in different order. The if and when clauses both succeed:
CoverageIfChooseIssue01.xspec calls template "IfThenChoose" and in the trace from 3_0_3 you only see "if" and in the coverage report the choose on lines 14 and 21 are set as missed.
CoverageIfChooseIssue02.xspec calls template "ChooseThenIf" and in the trace from 3_0_3 you only see "choose" and in the coverage report the if on lines 36 and 38 are set as missed.
This is a problem using the uqname check and would need a Saxonica fix (unless, as a special case, you wanted to consider if/choose the same thing).

With your current changes everything is good.

I'm not in any hurry for a response as I'm getting on ok at the moment using your changes and I've also set the tree model to linked in the Saxon config file.

CoverageIfChooseIssue.zip

[galtm]

Thanks. I'm looking at this now, in fact.

By the way, I edited your description at the top of this issue, to replace "1" with "2" in two spots ("CoverageGlobalVariableIssue02.xspec invokes TestTemplate02." instead of "CoverageGlobalVariableIssue01.xspec invokes TestTemplate02.", and similar in the last sentence.) I would not normally edit another person's comments, but I think these were merely copy/paste errors that are clearer to fix in place, for the sake of anyone else reading this issue.

[galtm]

I think the starting point should be a hit for everything that Saxon says is a hit, otherwise things get very tricky if you decide to disagree with Saxon and ignore some hits.

I agree.

1. Do you have a list of elements that are not in the trace output even when they are hit (and in what circumstances this happens)?
2. Do you have a written set of criteria that are used to define the situations when a node is hit/miss/ignored if it is not marked as hit in coverage.xml?
   Are the criteria tied down to the minimum use case?
   Do the criteria overlap in any ways such that there is a defined order in which they need to be applied?

You are asking good questions. XSpec doesn't have a formal specification, and I am not aware of artifacts that would answer your questions. I suspect that some of the XSLT logic was tailored for an older Saxon version that reported hits differently from Saxon 12.

I removed the 2 variable tests...

I think you are right to question those clauses. Perhaps if a past Saxon version never reported variable declarations as hit, it made sense for the coverage XSLT to look at a following sibling of a local variable. Looking at a following sibling of a global variable doesn't seem like the right logic.

I still see a problem for a global xsl:variable which has a select attribute with a string value because they are not hit in the trace when used. So I would say it should be marked as missed, but this is wrong and I don't know how you can get around this without getting Saxonica to change and include it in the trace.

Interesting observation, and it makes the task a little trickier.

The comment for this rule
<xsl:when test="empty(ancestor::xsl:*[parent::xsl:stylesheet or parent::xsl:transform])">ignored</xsl:when>
says <!-- A node within a top-level non-XSLT element -->
but is the test having unintended consequences, like my global param being ignored.

I agree, the comment doesn't match the logic, because that clause also matches top-level elements (including top-level XSLT elements) that don't match an earlier xsl:when clause.

Finally, my suggestion would be a review of the rules in xsl:choose in the element(0 | text() template to see if they are still doing that they are intended to do

Agreed.

Note: what I found useful in the testing I did was something like a version of the coverage-report that only reports the hits from the coverage.xml file (this made it easy to see what was and wasn't being traced which is how I got some of the suggestions above). I had to knock something up to do this but I assume you have something similar.

That's a good idea, and I can make something similar while investigating this.

With your change I think this raises a question about whether it is worth doing the comparison with the uqname at all because if it fails you will still record a hit if there is a class name, which seems to make the uqname check redundant

Yes, I also wondered whether we should merely pass along any hit with matching line/column data, regardless of the uqname or class name. That's not a change I want to do hastily, though. :)

From your comment, "I generated coverage reports for files in this repo's test suite and noticed ..." does this mean that you have automated checking of coverage reports against your current baseline?

Not exactly. The automated testing for coverage reporting in this repo is not as strong as the automated testing for the main XSpec functionality. I made an ad hoc script that generates lots of coverage reports, ran the script in workspaces with and without my code changes, and used a diff tool. Not nearly as automated as I would like.

Re #1921 (comment), I haven't looked at your CoverageIfChooseIssue.zip file yet.

General comment: What I'd like to do is treat these coverage reporting bugs as part of a larger task that involves enhancing automated testing of code coverage reporting in this repo, determining what's correct and what's incorrect, documenting the assumptions that this repo's code makes about what Saxon is reporting, and documenting the logic for when a node that Saxon doesn't report as "hit" should be marked as "missed" or "ignored."

I am happy to hear that you have workarounds at the moment, because I think being able to "refactor without fear" in this corner of the repo is going to require a fair amount of time to gradually build up foundations.

[birdya22]

I've come across another issue which I'll raise with Saxonica direct and let you know what the number is.
There doesn't seem much point in me providing you with a test case and then you raising it with Saxonica (you can always add a comment to the issue).
Basically when you have a match template with a union of nodes the Saxon trace is odd if you don't match the first node in the union e.g.
<template match="nodeA | nodeB">...</template>
When the template is invoked with nodeB you get 0 in some column numbers of the descendants (although I also saw a column number of 23 when I had xsl:apply-templates as a descendant - not the correct number) but the trace also says it matched nodeA.
When the template is invoked with nodeA the trace looks good.

Separately I have put together a stylesheet that includes a large majority of the xslt elements in some simple examples. I need to do more work on it but will share it with you later.

Adrian

[birdya22]

My new Saxonica issue is https://saxonica.plan.io/issues/6419.
I also noticed that Michael has responded to https://saxonica.plan.io/issues/6415#change-26038.

[galtm]

My new Saxonica issue is https://saxonica.plan.io/issues/6419.
I also noticed that Michael has responded to https://saxonica.plan.io/issues/6415#change-26038.

Thanks for reporting the new issue, and I see that Michael has already marked it as Resolved.

Separately I have put together a stylesheet that includes a large majority of the xslt elements in some simple examples. I need to do more work on it but will share it with you later.

Fantastic! It sounds like your work can lead to some test cases in this repo. Recent pull request #1923 sets up a structure for code coverage reporting test cases. I invite you to make pull requests with test cases, if/when you are ready (probably after #1923 is merged in). If you want to share your files but have someone else shepherd them through GitHub, that is fine, too.

[birdya22]

GitHub is a mystery to me (I only got an account so I could raise an issue).
See the attached - there is a ReadMe in there.
Can you have a look and let me know what you think - there is quite a lot to digest.
I've provided some thoughts on rules for the elements as well as a test case that covers most of the xslt elements.

XSLTTestExample.zip

Adrian

[birdya22]

I meant to mention about the template variables that are sometimes hit and sometimes missed even though they are all executed. I had a thought about optimisation and tried disabling all optimisations except j (-opt:j). This changed most variables from miss to hit. I also tried it with removing "inline variables" optimisation (-opt:-v)- this was nearly as good.
This is something else to consider, but there is a downside in this disclaimer "Disabling optimizations may in some cases prevent streamed evaluation. For example, some expressions only become streamable because the optimizer is able to work out that sorting nodes into document order is unnecessary."

[galtm]

Can you have a look and let me know what you think - there is quite a lot to digest.

Quick look so far -- this looks very, very useful for advancing the testing of the code coverage reporting feature. I will spend more time digesting it, hopefully this week. (I'll be away from computers much of next week.)

Is ../Input/Test01Data.xml referenced from MainStylesheet.xspec supposed to be in the ZIP-file?

[birdya22]

Is ../Input/Test01Data.xml referenced from MainStylesheet.xspec supposed to be in the ZIP-file?

Sorry yes - rename the text file.

I have noticed some issues with some individual tests and although I tried to make them all independent there is some interaction between them, and also having them all in one 1000 line stylesheet means it is easy to get confused. So I think it needs to be split up into small tests for each element to make them easier to understand/check/amend, although there could still be an overarching stylesheet/xspec that includes them all.

If you think this is a good idea I am ok with doing it and would suggest something like the following for a naming convention, unless you already have one:
xsl-element-name-01-c.xsl or xspec. The 01 is in case there are more tests for the element and the -c is to indicate a coverage test (just to make it easier to see when looking at the names). So a couple of examples would be:
xsl-accumulator-01-c.xsl and xsl-accumulator-01-c.xspec
xsl-where-populated-01-c.xsl and xsl-where-populated-01-c.xspec.

Test01Data.txt

[galtm]

rename the text file.

Got it. Thanks!

Small tests sound like a good idea for the reasons you mentioned.

The naming convention you proposed sounds good, though I think you can omit the -c part of the name because #1923 proposes a dedicated directory (test/end-to-end/cases-coverage/) for code coverage reporting tests. Initially, the xsl- prefix will be repetitive across the set of files in that directory, but the prefix will be quite useful if XSpec ever supports code coverage reporting for a language other than XSLT. I don't have a strong opinion about the xsl- prefix, so whatever you like is fine with me.

[galtm]

I merged #1923 onto the master branch. In case you find it helpful to see the test arrangement for code coverage reporting tests, check out these resources:

• README file for end-to-end testing
• New cases-coverage/ directory. So far, it has some tests that were previously in a sibling directory. I am thinking that your new tests could go in this directory, too.
• How to Run the Test Suite Locally

[birdya22]

I've started having a look at the links above.

Hopefully a quick question: in my xsl:character-map test the *-result.html output has 3 backward question marks each in a box rather than 100 (Saxon output produces 100). I assume I'm missing a bit of configuration in xspec - do you know what I need to do?

[galtm]

I'm not sure yet. Where in the report do those characters appear? Are they literally in the raw HTML (it's not the ⸮ character, ⸮, is it?) or are there some characters your browser can't display and it's showing the backward question marks instead? How can I reproduce the problem?

[birdya22]

I've attached my test case. It uses a character-map hierarchy and maps E000 to 0 and E001 to 1 to produce 100 in the output.
It's not that important at the moment.

characterMapIssue.zip

[galtm]

I don't think this is a configuration question after all. If you run the XSLT transformation outside XSpec, Saxon applies the character map using <xsl:output> when serializing the result for you. However, XSpec isn't actually serializing the output using <xsl:output>, so the weird characters you see are how the browser renders the original E000 and E001 characters. I found this out by executing the XPath expression //result//Q{}root/Q{}node/string() => string-to-codepoints() against the .../characterMapIssue/xspec/xsl-character-map-01-result.xml file I got from running the test, and then converting between decimal and hexadecimal.

Using the test attribute of <x:expect>, you can manually make XSpec serialize the result using a character map of your choice, and then re-parse for comparison with the expected result.

<x:expect label="Success"
   test=".
   => serialize(
      map {
         'method': 'xml',
         'use-character-maps': map{'&#xE000;': '0', '&#xE001;': '1'}
      }
   )
   => parse-xml()"
   select="/">
   <root>
      <node type="character-map">100</node>
   </root>
</x:expect>

It isn't pretty, although the details could potentially be hidden away in a helper stylesheet.

The technique I used is similar to the one in #227 (comment) .

[galtm]

From a code coverage perspective, the machinations to make XSpec serialize and reparse the result are not helpful, because they don't tell you whether running the transformation outside XSpec will "hit" or use the character map. Would making the XSLT use a character map in a serialize() expression cause XSpec to apply the character map without extra code on the XSpec side? Or does that not work because serialize() expresses the character map differently instead of referring to xsl:character-map?

[galtm]

It looks like you've already noted that Saxon does not trace character maps, even when it uses them. But if you need a way in XSpec to exercise a character map without extra XSpec code afterward, you could modify your match="xsl-character-map" template to call transform() with the option 'delivery-format': 'serialized'. Then, the XSpec file you attached in #1921 (comment) works fine. Does this help?

xsl-character-map-01-with-transform.xsl.zip

[birdya22]

I'm going for the very simple option, unless you would prefer me to do something else. My very simple option is:
<xsl:output-character character="0" string="0" />
Use the same value in the character and string attributes (for both 0 and 1). This gets xspec to work and I'm ok with Saxon working.

Also, I've raised another issue with Saxonica 6428 called "Output of xsl:on-non-empty changes when using TraceListener".
Basically you get different results from xsl:on-non-empty when you switch trace on and your using a sequence constructor for xsl:on-non-empty.
I haven't checked if anything else is affected like xsl:on-empty or xsl:where-populated as I'm sure Saxonica will investigate.
Adrian

[galtm]

Simplicity is good, and so is reporting an issue you found along the way. Maybe Saxon 13 will be a great release for tracing.

[galtm]

Adrian, I looked at your ZIP-file a little more, and it is really a wealth of information. I have some questions:

1. Do you think "Column=0 in xspec and Saxon trace." in xsl:evaluate reflects a Saxon bug?

2. Do you think it's reasonable to request that Saxon include xsl:perform-sort in its trace data?

3. Do you think "Column=0 in xspec trace. Not in Saxon trace." in xsl:assert, xsl:on-non-empty, and xsl:try reflects an XSpec bug?

4. Can you clarify your rule for xsl:param, "if global (parent is xsl:stylesheet or xsl:transform) or parent is xsl:template miss."? Are you saying that Saxon traces global params and template params, so the rule should be "mark as hit if Saxon says it's hit, and mark as missed otherwise"?

5. If we add a new "unknown" category to the report, we'll need to explain to users how it differs from the "ignore" category. Are you thinking that "ignore" is a design decision that can stand on its own logic, whereas "unknown" is a result of insufficient information?

[birdya22]

As you're not around next week I'll have a think about your questions and reply later.
I've nearly got the individual test cases done and will look to run them through "Running the Test Suite Locally" next week.
I'm also going to try and put together some information about the impact, or not, or using the linked tree and turning optimization off, but I probably need some different test cases because the MainStylesheet.xsl and my individual test cases don't reflect real use and probably won't be affected very much by the linked tree and optimization changes.
Adrian

[birdya22]

Starting with replies to your questions.

Do you think "Column=0 in xspec and Saxon trace." in xsl:evaluate reflects a Saxon bug?

Yes although from the comments from Michael Kay I'm not sure Saxonica will do anything about it until version 13 when it all might change.

Do you think it's reasonable to request that Saxon include xsl:perform-sort in its trace data?

I would say yes because it is an instruction.

Do you think "Column=0 in xspec trace. Not in Saxon trace." in xsl:assert, xsl:on-non-empty, and xsl:try reflects an XSpec bug?

There is a change in my understanding of xsl:assert and xsl:try (see the XML file) as the TryCatch class trace appears on the line after the xsl:try and depends on what the first element is in xsl:try.
But, no I don't think it is an XSpec bug.

Can you clarify your rule for xsl:param, "if global (parent is xsl:stylesheet or xsl:transform) or parent is xsl:template miss."? Are you saying that Saxon traces global params and template params, so the rule should be "mark as hit if Saxon says it's hit, and mark as missed otherwise"?

Yes.

If we add a new "unknown" category to the report, we'll need to explain to users how it differs from the "ignore" category. Are you thinking that "ignore" is a design decision that can stand on its own logic, whereas "unknown" is a result of insufficient information?

Basically yes. From my tests I think "unknown" will be for elements that use a select attribute and are not traced. If the elements has a sequence constructor it is possible to use the state of the children to determine the state of the parent. I added some more tests in for this and the results are in the updates to the XSLT file.

Here are my thoughts on Optimization and Tree Models.

Optimization

Having played with a bit of Java I've discovered that HE only supports the following optimization values: lmtv.
From a bit of testing, changing the values seems to just affect xsl:variable (but there may be other things as well).
See https://saxonica.plan.io/issues/6415 for a Saxonica comment on tracing and optimization.
With optimization turned on the trace can miss variables that are used.
With optimization turned off the trace can hit variables that are not used.
So neither option is great.
I've don't know what happens with PE or EE.

Optimization would need to be configured in the "Running Tests" part where users could change it by setting a value in the SAXON_CUSTOM_OPTIONS environment variable (this would override a setting in a new saxon config file that could be used although you could set it as a command line option after the SAXON_CUSTOM_OPTIONS).
My suggestion would be to stick with the default optimization and add a warning on the Code Coverage Wiki page that some elements may not appear to be hit because of optimization and possibly describe how to see what optimization does rather than try and explain why variables are hit when there is no optimization (outside of xspec they can set the config item traceOptimizerDecisions to display what Saxon does during optimization. Some examples of the optimization are "Eliminated trivial variable", "Eliminated unused variable", "Inlined constant variable" and these are what causes issues with xsl:variables traced or not).
This also eliminates the Saxonica warning that disabling optimization can affect streaming.

Tree Model

Based on #1920 and https://saxonica.plan.io/issues/6405 with the fact that using the LinkedTree when generating the Coverage Report gives better column number values my suggestion is that the coverage-report-config.xml file has the treeModel="LinkedTree" set in it (I can provide details of the difference it makes to my test set if you want).

Are you thinking about a new release of XSpec before a new release of Saxon or waiting until after the next release?

[birdya22]

It would help if I attached something.
20240526Update.zip

[galtm]

Thanks a lot for your comments and all the files and information in 20240526Update.zip. And thanks for your patience with my turnaround time. I am just starting to look at the latest ZIP-file.

Are you thinking about a new release of XSpec before a new release of Saxon or waiting until after the next release?

XSpec doesn't have a history of very frequent releases, and the tentative date for the next release is in the fall. Saxonica has been fixing things that affect code coverage reporting in XSpec, yet I haven't seen all the fixes and don't know if XSpec needs further adaptations (e.g., in the custom listener or the report XSLT) to benefit from the Saxon fixes. My inclination is to have the next XSpec release wait at least until Saxon 12.5 but not necessarily wait for Saxon 13.

[galtm]

I've reviewed about half of your per-element coverage tests so far. I think they will fit nicely in the test/end-to-end/cases-coverage harness. If we get the tests in place first, they will help immensely when we need to evaluate the effects of either a change in Saxon or a change in this repo's code (e.g., coverage-report.xsl or Saxon configuration).

Although I'm not done reviewing your tests as they run against the master branch, I peeked ahead to see how the change in #1918 affects the results. In some cases, it is a matter of text nodes being marked as missed on master and correctly marked as hit on the #1918 branch. There was also a case in xsl-comment-01.xspec where swapping code blocks in XSLT affected the correctness of the coverage report on the master branch, but the report was correct either way on the #1918 branch. So, your tests are helping give me more confidence about #1918 being on the right track (at least with Saxon 12.4).

I have suggestions for resolving the open items you flagged in external_xsl-global-context-01.xsl, xsl-assert-01.xsl, xsl-evaluate-01.xsl, and external_xsl-all-01.xspec.

You mentioned GitHub being a mystery, so if it is OK with you, I'll create a pull request with your files plus my modifications. We can use the pull request page to continue the discussion if we have line-specific comments to discuss.

To attribute your work to you, should I include your name and GitHub user name in the descriptions of commits that contain your code and in the pull request description? Let me know what you're comfortable with as far as visibility and privacy go. If you decide you want to make the commits and pull request yourself, let me know!

[galtm]

@birdya22 : I've reviewed all your per-element coverage tests and assessed the effects of #1918 on the results. I'll wait to hear from you about attribution before setting up a pull request with the per-element coverage tests. (I could upload a ZIP-file so you could see the files, but then you wouldn't have the Git commits' comments and groupings.)

In the meantime, there are parts of this conversation I still need to digest that would feed into subsequent pull requests.

[birdya22]

You mentioned GitHub being a mystery, so if it is OK with you, I'll create a pull request with your files plus my modifications.

Yes.

To attribute your work to you, should I include your name and GitHub user name in the descriptions of commits that contain your code and in the pull request description? Let me know what you're comfortable with as far as visibility and privacy go.

I'm ok with that.