-
Notifications
You must be signed in to change notification settings - Fork 31
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
Support for SAXON_CUSTOM_OPTIONS in command line #268
Conversation
…axon-custom-options # Conflicts: # test/xspec-bat.cmd # test/xspec.bats
…om-options # Conflicts: # test/xspec-bat.cmd # test/xspec.bats
…axon-custom-options # Conflicts: # test/xspec.bats
…om-options # Conflicts: # test/xspec.bats
…axon-custom-options
…om-options # Conflicts: # test/xspec-bat.cmd # test/xspec.bats
…axon-custom-options # Conflicts: # test/xspec-bat.cmd # test/xspec.bats
…axon-custom-options
…axon-custom-options
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me. I reviewed it as follows:
-
I ran successfully the bats tests locally on Linux and Windows and made sure that the new tests are executed ('invoking xspec.sh for XSLT with SAXON_CUSTOM_OPTIONS' and 'invoking xspec.sh for XQuery with SAXON_CUSTOM_OPTIONS').
-
I made a test failing on purpose to check that errors are caught. It failed as expected.
-
I inspected the code and read the discussion in Is it possible to apply Saxon configuration files? #235.
-
Since @duenckel successfully tested it manually with xspec.bat, I tested it manually with xspec.sh in bash. First, I set up the environment variable:
export SAXON_CUSTOM_OPTIONS='"-config:test/saxon-custom-options/config.xml" -t'
Then I ran successfully the test:
bin/xspec.sh test/saxon-custom-options/test.xspec
I also tested that this works with SAXON EE. To do this, in
test/saxon-custom-options/config.xml
I changed the edition attribute toedition="EE"
and added the serialization configuration which is supported only by Saxon EE or PE:<serialization method="xml" indent="yes" saxon:indent-spaces="8" xmlns:saxon="http://saxon.sf.net/"/>
The test ran successfully with SAXON EE and I also simulated an error. Since we can't run tests on the CI systems with SAXON EE at the moment, we can't add this additional test to the test suite (but I wanted to check it manually any way).
@AirQuick: As soon as you rebase the branch of this pull request on the current master
(I pushed a commit to master since your last commit), I will merge this into master
.
Just a question (which does not impact the merge): Saxon custom options are now supported when running XSpec for XSLT and XQuery. Would we need something similar for running XSpec for Schematron? Or is it not relevant for the Schematron support?
…om-options # Conflicts: # test/xspec-bat.cmd # test/xspec.bats
Since XSpec for Schematron is eventually converted to XSpec for XSLT, we don't need to add anything. Saxon custom options take effect when testing Schematron just fine. (And, for the same reason, I don't think we need a dedicated test.) |
Added the variable to Wiki. |
Thanks for the clarification and for updating the documentation. |
This pull request derives from #267 . So needs to be handled after that.
Closes #235
This pull request mimics the
saxon.custom.options
property of Ant.With it, you can set the environment variable
SAXON_CUSTOM_OPTIONS
before runningxspec.bat
orxspec.sh
. For example, if you door
and then
-config:...
and-t
are passed to Saxon and take effect when running the test intest.xspec
.(The options do not take effect when compiling the test or formatting the test result, which is the same as Ant.)