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

Basic test for code coverage (Ant) #300

Merged
merged 134 commits into from
Dec 26, 2018
Merged

Conversation

AirQuick
Copy link
Member

@AirQuick AirQuick commented Aug 22, 2018

This pull request derives from #299. So needs to be handled after that.


This pull request adds some basic tests for the code coverage on Ant (implemented in #279).

Some of the tests are enabled only when Saxon (specified by SAXON_JAR) is capable of the code coverage. So they are skipped on Travis and AppVeyor. They are enabled automatically, if you're running the tests on your machine with Saxon license.

How it works

  • run-bats.sh (and .cmd) checks to see if SAXON_JAR can compile the coverage reporter XSL.
    If it can, the environment variable XSLT_SUPPORTS_COVERAGE is set. (d557f39)
  • xspec.bats (and collection.xml) checks XSLT_SUPPORTS_COVERAGE before testing the coverage report feature. (01e6386)

How to test

Here's what I tested on my local machine.

  1. Do set SAXON_JAR=\path\to\saxon9ee.jar. (saxon9pe.jar should be fine too, but I haven't tested.)

  2. Run run-bats.cmd in test/.
    Must pass, including the test case 47.

  3. Do set SAXON_JAR=\path\to\saxon9he.jar. (Notice the file name; It's Saxon-HE.)

  4. Run run-bats.cmd.
    Must pass, skipping the test case 47. (demonstrated on Travis and AppVeyor)

  5. Check out 024c845 which is intentionally broken.

  6. Do set SAXON_JAR=\path\to\saxon9ee.jar.

  7. Run run-bats.cmd.
    At least 29, 47, 48 and 49 must fail. (29, 48 and 49 are demonstrated on Travis and AppVeyor)

Repeat the same steps using run-bats.sh --tap on Ubuntu.

…-tests

# Conflicts:
#	build.xml
#	test/run-xspec-tests-ant.cmd
#	test/run-xspec-tests-ant.sh
…-tests

# Conflicts:
#	test/end-to-end/generate-expected.cmd
#	test/end-to-end/generate-expected.sh
#	test/end-to-end/run-e2e-tests.cmd
#	test/end-to-end/run-e2e-tests.sh
…overage

# Conflicts:
#	build.xml
#	test/end-to-end/generate-expected.cmd
#	test/end-to-end/generate-expected.sh
#	test/end-to-end/processor/html/_normalizer.xsl
#	test/end-to-end/run-e2e-tests.cmd
#	test/end-to-end/run-e2e-tests.sh
#	test/run-xspec-tests-ant.cmd
#	test/run-xspec-tests-ant.sh
…185_cumulative

# Conflicts:
#	test/xspec-bat.cmd
#	test/xspec.bats
…-tests

# Conflicts:
#	test/xspec-bat.cmd
#	test/xspec.bats
…overage

# Conflicts:
#	test/xspec-bat.cmd
#	test/xspec.bats
…k/xspec into align-bats-tests

# Conflicts:
#	test/xspec-bat.cmd
#	test/xspec.bats
…-tests

# Conflicts:
#	test/xspec-bat.cmd
#	test/xspec.bats
…into test_ant-coverage

# Conflicts:
#	test/xspec.bats
…overage

# Conflicts:
#	test/run-bats.cmd
#	test/run-bats.sh
#	test/win-bats/collection.xml
#	test/xspec.bats
@AirQuick AirQuick added this to the v1.2.0 milestone Dec 21, 2018
…overage

# Conflicts:
#	.travis.yml
#	appveyor.yml
#	test/win-bats/collection.xml
#	test/xspec.bats
Copy link
Member

@cirulls cirulls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm having issues testing this on my machine but I think it is due to my configuration rather than to this pull request. For some reason when I run run-bats.sh with Saxon EE the Saxon license file (which is in the same directory as the Saxon EE jar file) is not picked up and test 40 gives me this output:

   Creating Test Stylesheet...
   No license file found - running with licensable features disabled
   
   Running Tests...
   No license file found - running with licensable features disabled
   Testing with SAXON EE (unlicensed) 9.6.0.4

Test 47 is always skipped on my local machine and I think it is for a similar reason. However, if I run the test manually from the test directory with this command:

ant -buildfile "${PWD}/../build.xml" -Dxspec.xml="${PWD}/../tutorial/coverage/demo.xspec" -lib "${SAXON_JAR}" -Dxspec.coverage.enabled=true

the report is correctly created. I also checked that my license file for Saxon EE is valid and found by running these commands.

I wonder if you encountered a similar issue in the past or if you had to do any special set up for Saxon EE in order to pick up the license file correctly when using the shell and batch scripts. If possible, could you share your configuration for Saxon EE? It may help me setting up mine.

On another subject: the run-bats.sh contains this command:

if java -jar "${SAXON_JAR}" -nogo -xsl:../src/reporter/coverage-report.xsl 2> /dev/null;

I looked for documentation in Java and Saxon for the option -nogo but I couldn't find anything. What does this option do?

@AirQuick
Copy link
Member Author

AirQuick commented Dec 26, 2018

  • Test 40 copies the Saxon jar but it doesn't copy the license file. That's why Saxon 9.6 emits No license found. Saxon 9.7 or later does not emit this warning in this particular case.
    (That being said, Saxon may emit the warning again in the future. To avoid it, I committed f8eef90 to copy the license file. Now the test 40 should pass on Saxon 9.6.)
  • -nogo was introduced on Saxon 9.7. So the test in this pull request currently requires 9.7 or later. That's why Test 47 is skipped on 9.6.

@cirulls
Copy link
Member

cirulls commented Dec 26, 2018

Test 40: thanks for the additional commit, I confirm it works fine with Saxon EE 9.6 and the test is executed and passes.

Test 47: thanks for the explanation about the -nogo option, I learnt something new! I am aware that we should be targeting modern versions of Saxon but I wonder if it would be possible to provide some backward compatibility for Saxon EE 9.6, this would allow me to run locally tests requiring schema aware versions of Saxon (unfortunately I am unlikely to buy a new version of Saxon EE).

In order to test if Saxon EE 9.6 was actually capable of running test 47, I removed the if condition from this statement:

if java -jar "${SAXON_JAR}" -nogo -xsl:../src/reporter/coverage-report.xsl 2> /dev/null; then
 	export XSLT_SUPPORTS_COVERAGE=1
 fi

so that XSLT_SUPPORTS_COVERAGE is always set to 1. I ran run-bats.sh on my machine with Saxon EE 9.6 and all the tests (including test 47) were executed successfully. I also double checked that the coverage report was actually generated in test 47. This makes me think that even Saxon EE 9.6 is capable of running that test.

To find a backward compatible solution I reinstated the if condition covering Saxon EE 9.7 and added this snippet in run-bats.sh for Saxon EE/PE 9.6:

saxon_version=$(java -jar "${SAXON_JAR}" -t 2>&1 | head -n 1)
if [[ "${saxon_version}" =~ ^Saxon-[E|P]E ]]; then
    export XSLT_SUPPORTS_COVERAGE=1
fi

saxon_version stores the version of Saxon and the if condition checks if the string stored in saxon_version starts with Saxon EE or Saxon PE. I checked that the test suite passes on Linux and the test is skipped when running it with Saxon HE. To make the test even stricter, it is possible to match a longer string (the value of saxon_version in my set up is Saxon-EE 9.6.0.4J from Saxonica).

I wonder if a snippet like this (or a similar implementation, I am not too fussy about is as long as I can run it on my machine) could be added to your pull request. I am aware that the if condition with the -nogo option is stricter because it actually checks that the stylesheet can be compiled instead of blandly assuming that Saxon EE/PE will do the job. However, adding an extra condition like this one would allow me (and maybe other contributors) to run this type of tests locally.

Let me know what you think and feel free to suggest another solution.

@AirQuick
Copy link
Member Author

Checking the Saxon version output won't last long.
For example, in #327, the test requires a schema-aware XSLT processor. Saxon-EE has actually 4 variants: EE, EE-V, EE-T, EE-Q. The version output doesn't tell the variant.
Also, the code coverage doesn't work on Saxon 9.4. To tell it, you need to parse not only the edition but also the version number.
Further more, in #340 and #341, the schema-aware scenarios and the code coverage are tested in the end-to-end test. You need to tell Saxon's capability in Ant.
So... Things get complicated, if you rely on the version output. Trying actually running something simplifies all.

Fortunately, the test script doesn't clear the environment variable. You can force running the coverage test.

export XSLT_SUPPORTS_COVERAGE=1
./run-bats.sh
set XSLT_SUPPORTS_COVERAGE=1
run-bats.cmd

This way, you can run the coverage test on Saxon 9.6 (or any other).

@cirulls
Copy link
Member

cirulls commented Dec 26, 2018

Thanks, your solution is a good workaround for me and allows me to run the tests without adding more complexity to the code base. I can now follow your procedure for testing and everything works as you described in both Linux and Windows.

I'm merging this pull request into master.

@cirulls cirulls merged commit cba4428 into xspec:master Dec 26, 2018
@AirQuick AirQuick deleted the test_ant-coverage branch December 26, 2018 23:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants