Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Content of role attributes #665

Closed
heidivanparys opened this issue Oct 2, 2019 · 12 comments · Fixed by #705
Closed

Content of role attributes #665

heidivanparys opened this issue Oct 2, 2019 · 12 comments · Fixed by #705

Comments

@heidivanparys
Copy link

When creating my first tests using XSpec, I tested a Schematron file that used asserts with role attribute set to ERROR, as described on http://schematron.com/2018/12/standard-severity-levels-with-schematron-role/ :

So here is a list of severity levels that I think Schematron tools and schemas might consider supporting. It is drawn from various IDEs and the Language Server Protocol. I have added the equivalent LSP severity levels, and potential ISB severity levels.

    @role="FATAL"  – something so bad that processing or validation should or did stop.
        LSP Error
        ISQB S1 CRITICAL
    @role="ERROR"  – something  wrong has occurred but processing may continue
        LSP Error
        ISQB S2 MAJOR
    @role="WARN"  – something wrong has happened, but it does not necessarily require action
        LSP Warning
        ISQB S3 MINOR
    @role="CAUTION"  – It may not be wrong, but care is required. Or a system might be expected to process the document in a special way because of this.
        LSP Information
        ISQB S4 TRIVIAL
    @role="INFO"  – some information is being reported
        LSP Information
    @role="HINT"  – some hint is being given to the user
        LSP Hint
    @role="TRACE"  – some information on execution is being reported.
        LSP Information
    @role="DEBUG"  – some information that is not intended for exposure in production.
        LSP Hint

These severities also represent an order in which SVRL items might be presented to the user, if useful.

This resulted in x:expect-valid not working as I expected it to work.

When looking at the xspec code, I then found out that lowercase roles are used:

As a minimum, I think an update of https://github.com/xspec/xspec/wiki/Writing-Scenarios-for-Schematron is needed. Currently is says:

x:expect-valid verifies that the Schematron is executed and passes validation. In the Schematron an assert or report can have a role attribute specifying that it is a warning or informational message and these are considered to be allowed for a passing validation.

This could e.g. be changed to:

x:expect-valid verifies that the Schematron is executed and passes validation. In the Schematron an assert or report can have a role attribute specifying that it is an error (value 'error' or 'fatal'), a warning (value 'warn' or 'warning') or an informational message (value 'info' or 'information'). A role attribute not equal to 'error' or 'fatal' is considered to be allowed for a passing validation.

A more advanced solution could be to extend the values XSpec can recognize (lowercase vs. uppercase, and the addition of the values described at http://schematron.com/2018/12/standard-severity-levels-with-schematron-role/ )

@AirQuick AirQuick added this to the v1.5.0 milestone Oct 2, 2019
@AirQuick
Copy link
Member

AirQuick commented Oct 2, 2019

@vincentml
What do you think?

@vincentml
Copy link
Collaborator

vincentml commented Oct 3, 2019

The current list of @role values 'error' and 'fatal' is based on the behavior of oXygen. I agree it does make sense to change the definition of the $error variable to include FATAL and ERROR as written in the schematron.com blog post. For completeness $warn and $info can also be updated (I think $warn and $info are not currently used in XSpec).

    <xsl:variable name="error" select="('error', 'fatal', 'ERROR', 'FATAL')"/>
    <xsl:variable name="warn" select="('warn', 'warning', 'WARN', 'CAUTION')"/>
    <xsl:variable name="info" select="('info', 'information', 'INFO', 'HINT')"/>

The suggested documentation change is also a good improvement. In conjunction with the code change, the text would change to include the new values. In this context it is probably also good to mention that role values are user defined. So the new text would be:

x:expect-valid verifies that the Schematron is executed and passes validation. In the Schematron an assert or report can have a role attribute specifying that it is an error (value 'error', 'fatal', 'ERROR' or 'FATAL'), a warning (value 'warn', 'warning', 'WARN', or 'CAUTION') or an informational message (value 'info', 'information', 'INFO', or 'HINT'), or potentially other user-specified values. A role attribute not equal to 'error', 'fatal', 'ERROR', or 'FATAL' is considered to be allowed for a passing validation.

@AirQuick
Copy link
Member

AirQuick commented Oct 4, 2019

Thanks @vincentml !

@heidivanparys
Are the suggested changes fine with you?

@heidivanparys
Copy link
Author

Thanks for looking at this issue. I have one comment: how about making all values case-insensitive (from a user-perpective)?:

    <xsl:variable name="error" select="('error', 'fatal', 'ERROR', 'FATAL')"/>
    <xsl:variable name="warn" select="('warn', 'warning', 'caution, 'WARN', 'WARNING', 'CAUTION')"/>
    <xsl:variable name="info" select="('info', 'information', 'hint', 'INFO', 'INFORMATION', 'HINT')"/>

The text in the wiki would then have to be updated accordingly.

@vincentml
Copy link
Collaborator

If the values were compared in a case-insensitive way then XSpec may be more forgiving of typos than might be expected by a user or some other environment in which Schematron runs. For example, "erROR" would probably be considered incorrect. I think it is better to have a defined list of all lower case and all upper case versions of each value to encourage consistency, but an argument could be made that a case insensitive comparison would be more user friendly.

Researching this, the logic that oXygen uses to determine severity is documented here:

When editing Schematron files in oXygen the suggested values for the role attribute are:

  • error
  • fatal
  • info
  • information
  • warn
  • warning

A quick test in oXygen of all upper case, all lower case, and mixed-case variants shows that oXygen does use a truly case-insensitive comparison for the role attribute values.

To match oXygen's treatment of the role attribute values I think the variables that we've been discussing should be defined using all lower case values, e.g.

<xsl:variable name="error" select="('error', 'fatal')"/>

The code generation for comparison of the role attribute value in schut-to-xspec.xsl#L183 can be changed to be case insensitive by:

not(@role) or lower-case(@role) = (',

@AirQuick
Copy link
Member

AirQuick commented Oct 11, 2019

@heidivanparys or @vincentml
Aligning with oXygen sounds like a very good idea. It'll meet the majority of expectations.
Either of you are kindly willing to make a pull request? Making it would be as easy as these two steps:

  1. Create a set of .xspec and .sch files which fails when used with the current master branch, and put the files into test/ directory
  2. Implement the change in schut-to-xspec.xsl

@heidivanparys
Copy link
Author

I could create a pull request. I won't manage to do that this week but next week should be possible.

@heidivanparys
Copy link
Author

I could create a pull request.

@AirQuick I'm making only little progress with this unfortunately.

I followed https://github.com/xspec/xspec/wiki/How-to-Run-the-Test-Suite-Locally . I managed to install the dependencies correctly by using a modified version of test\ci\install-deps.cmd , as I have an older Windows version, see heidivanparys@ddb721e

Then I created a Schematron file, an XSpec test, and did an attempt to describe (part of) the expected outcome, see heidivanparys@5e286e0

I cannot get what is described at https://github.com/xspec/xspec/tree/master/test/end-to-end to work, I get an error:

xspec.preprocess-schematron-xspec:
BUILD FAILED
C:\Users\<myUsername>\Documents\GitHub\heidivanparys\xspec\test\end-to-end\ant\base\build.xml:19: The following error occurred while executing this line:
C:\Users\<myUsername>\Documents\GitHub\heidivanparys\xspec\test\ant\build.xml:107: The following error occurred while executing this line:
C:\Users\<myUsername>\Documents\GitHub\heidivanparys\xspec\test\end-to-end\ant\generate-expected\worker\build-worker.xml:69: The following error occurred while executing this line:
C:\Users\<myUsername>\Documents\GitHub\heidivanparys\xspec\test\end-to-end\ant\generate-expected\worker\build-worker.xml:73: The following error occurred while executing this line:
C:\Users\<myUsername>\Documents\GitHub\heidivanparys\xspec\test\end-to-end\ant\generate-expected\worker\build-worker.xml:21: The following error occurred while executing this line:
C:\Users\<myUsername>\Documents\GitHub\heidivanparys\xspec\test\end-to-end\ant\base\worker\build-worker.xml:44: The following error occurred while executing this line:
C:\Users\<myUsername>\Documents\GitHub\heidivanparys\xspec\build.xml:169: Fatal error during transformation using C:\Users\<myUsername>\Documents\GitHub\heidivanparys\xspec\test\end-to-end\processor\xml\normalize.xsl: An empty sequence is not allowed as the value of variable $input-tokens; SystemID: file:/C:/Users/B004114/Documents/GitHub/heidivanparys/xspec/test/end-to-end/processor/base/_normalizer.xsl; Line#: 24; Column#: 9

Would it be possible to get some more guidance on how many files and what kind of files should be added, and what the process is to create a complete test case?

@AirQuick
Copy link
Member

AirQuick commented Oct 28, 2019

@heidivanparys
Thanks for making the changes.
Using the current master, two scenarios error: expect-valid should fail and fatal: expect-valid should fail are Success. Once the required changes are made, those two scenarios should be Failure, right?

I managed to install the dependencies correctly by using a modified version of test\ci\install-deps.cmd , as I have an older Windows version, see heidivanparys/xspec@ddb721e

Thanks for making it. I'll try to incorporate it. (#692)

I cannot get what is described at https://github.com/xspec/xspec/tree/master/test/end-to-end to work

Looks like the test processor needs to be updated to handle this new pattern. I'll fix it asap. (#693)

@AirQuick
Copy link
Member

I cannot get what is described at https://github.com/xspec/xspec/tree/master/test/end-to-end to work

Looks like the test processor needs to be updated to handle this new pattern. I'll fix it asap. (#693)

Fixed it in #694. I believe the test will work on the current master.

@heidivanparys
Copy link
Author

Using the current master, two scenarios error: expect-valid should fail and fatal: expect-valid should fail are Success. Once the required changes are made, those two scenarios should be Failure, right?

Yes indeed.

I cannot get what is described at https://github.com/xspec/xspec/tree/master/test/end-to-end to work

Looks like the test processor needs to be updated to handle this new pattern. I'll fix it asap. (#693)

Fixed it in #694. I believe the test will work on the current master.

Thanks, that made sure I didn't get the same error anymore.

@AirQuick
Copy link
Member

AirQuick commented Nov 1, 2019

Assuming we're merging #705 soon, I updated Wiki based on @vincentml 's suggestion. (diff)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants