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

Add Ant build support for code coverage #279

Merged
merged 69 commits into from
Nov 26, 2018

Conversation

AirQuick
Copy link
Member

@AirQuick AirQuick commented Aug 9, 2018

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


Closes #147

This pull request allows to generate the code coverage report using Ant build script.
The new property xspec.coverage.enabled controls that. (default false)

How to use

Command line

  1. Clone this pull request.
  2. Just add -Dxspec.coverage.enabled=true to your usual command line. For example,
    ant -buildfile /path/to/xspec/build.xml -lib /path/to/saxon-ee.jar -Dxspec.xml=/path/to/your.xspec -Dxspec.coverage.enabled=true
    
    ==> *-coverage.html will be generated in the xspec subdirectory.

Oxygen

  1. Clone this pull request. For example, clone it into /test/xspec/.
  2. In Oxygen, open your .xspec file.
  3. In the Transformation Scenarios pane, duplicate Run XSpec Test.
  4. In the Options tab of Edit Ant Scenario dialog, specify /test/xspec/ in Working directory.
  5. In the Parameters tab, set xspec.project.dir = /test/xspec/
  6. Click New in the Parameters tab and add xspec.coverage.enabled = true.
  7. Click New in the Parameters tab and add xspec.coverage.html = ${cfd}/${cfn}-coverage.html.
  8. In the Output tab, set ${cfd}/${cfn}-coverage.html in Open.
  9. Click OK to close the Edit Ant Scenario dialog.
  10. Apply the new duplicated transformation scenario.
    ==> XSpec Ant begins collecting code coverage. Oxygen will open the coverage report in your default browser.

Test

Using the files in #197, I manually verified that this pull request generated the same HTML file as the current master.

No automated tests for now. It should be addressed by #195.

Further work

  • Document xspec.coverage.enabled in Wiki.
    (xspec.coverage.html should be kept undocumented unless xspec.result.html is documented...)

…ge_stderr

# Conflicts:
#	test/xspec-bat.cmd
#	test/xspec.bats
@cirulls cirulls added this to the v1.2.0 milestone Oct 18, 2018
…ge_stderr

# Conflicts:
#	test/saxon-custom-options/config.xml
#	test/saxon-custom-options/test.xspec
#	test/xspec-bat.cmd
#	test/xspec.bats
@AirQuick
Copy link
Member Author

Although labelled as enhancement, this PR is actually required for testing. The following tests depend on this PR.

So, in short, I hope this PR get merged asap and move forward :)

@AirQuick
Copy link
Member Author

@yamahito
This will close #147. Please let us know if you have a concern.

@cirulls
Copy link
Member

cirulls commented Nov 21, 2018

@AirQuick: this is the next pull request to review on my list. I'll try to review by the end of this week, please bear with me for few days.

@cirulls
Copy link
Member

cirulls commented Nov 21, 2018

@AirQuick: I tried to run it on the command line and on oXygen but I got inconsistent coverage reports, I'm not sure if it is an issue in my configuration and want to double check with you first.

Command line
Environment: Linux
Shell: bash
Ant version: 1.9.6
Saxon EE: 9.7
Command:

ant -buildfile build.xml -lib /home/cirulls/Documents/programming/saxon-9ee/saxon9ee.jar -Dxspec.xml=tutorial/escape-for-regex.xspec -Dxspec.coverage.enabled=true

I get this coverage report:

image

This is what I expected and it's the same coverage report I would get via xspec.sh -c. So far so good.

oXygen
oXygen version: 20.0
Ant version: 1.9.8 (default on oXygen 20.0)
Saxon EE: 9.8.0.8 (default on oXygen 20.0)

I followed your instructions for setting the transformation scenario, here is my setting:

image

image

image

However, I get a different coverage report:

image

I would expect the same report but instead I get no missing target (i.e. nothing is highlighted in red).

Could you double check if you get the same report at your end?

I initially suspected that Saxon EE was not picked up in my transformation scenario settings but a) I could not see any option in the transformation scenario to specify which Saxon processor to use; b) I would expect the transformation scenario to fail in case Saxon HE is used by default.

Can you see any obvious mistake on my oXygen settings? Could you share your configuration?

@AirQuick
Copy link
Member Author

That difference comes from the Saxon version difference.
Using the current master + command line, Saxon 9.6 and 9.8 behave differently in the same way as you found. It's my impression that the 9.8 output (and your oXygen output) is correct.

@AirQuick
Copy link
Member Author

And that's why I filed #196, submitted #197 (now #299) and used the files in it for testing this pull request.

@cirulls
Copy link
Member

cirulls commented Nov 22, 2018

Thanks for your comments, it makes sense now. I tried the same with the tutorial files for code coverage you committed in #299 and I got identical code coverage reports on both the command line and oXygen 20.0 so this is good.

I'll finish off my review and keep you posted, I expect to merge this soon into master.

@AirQuick
Copy link
Member Author

I'll merge this if no concern arises in the next 72 hours.

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.

This looks good to me. Sorry for the wait, I finished the review yesterday but didn't have time to post it. As discussed on the pull request thread, I was able to run this successfully and consistently on both the command line and oXygen 20 using the coverage demo files in #299. I also inspected the files changed and ran the test suite locally on Linux, all tests are green.

After #299 is merged and the demo files for code coverage are added to master, I think it would be useful to update the documentation for both Code Coverage and Ant (maybe adding a paragraph on how to run the code coverage via Ant). If you want, you can add now documentation for the parameter xspec.coverage.enabled in Ant.

I'm merging this into master.

@AlexJitianu and @yamahito: you may be interested in this for the oXygen harmonisation.

@cirulls cirulls merged commit 6f016ec into xspec:master Nov 26, 2018
@AirQuick AirQuick deleted the ant-coverage_stderr branch November 26, 2018 09:32
@AirQuick
Copy link
Member Author

Document xspec.coverage.enabled in Wiki.

Done:

@cirulls
Copy link
Member

cirulls commented Nov 26, 2018

Looks good, thanks for updating the documentation.

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

Successfully merging this pull request may close these issues.

Oxygen Harmonization: Code Coverage
2 participants