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

Align Travis configuration with AppVeyor in #86 #96

Merged
merged 3 commits into from Mar 7, 2017
Merged

Conversation

cirulls
Copy link
Member

@cirulls cirulls commented Mar 6, 2017

Summary

This pull request aligns the Travis configuration with the AppVeyor configuration implemented in #86.

Why

#76 introduced a test for checking that the XProc harness is working. Running XProc requires XMLCalabash which is installed in the .travis.yml file. However, XMLCalabash currently comes bundled with Saxon 9.7.0.11 as highlighted by @AirQuick in #86 (comment). It is therefore useless to install it and run the test for XProc when a lower version of Saxon is used. This also makes the test suite slower.

What has changed

In .travis.yml I added the XMLCALABASH_VERSION in the matrix as suggested by @AirQuick in #86 (comment). This is in line with the configuration in appveyor.yml and also makes it easier to upgrade to a new version of XMLCalabash in the future.

In xspec.bats I skipped the test if the environment variable XMLCALABASH_VERSION is not set.

How to review

I tested that the Travis configuration works as expected in this Travis build (see the job logs):

  • build 235.1 (Saxon 9.7.0-15) installs XMLCalabash and runs the test
  • build 235.2 (Saxon 9.6.0-10) doesn't install XMLCalabash and skip the test
  • build 235.3 (Saxon 9.6.0-7) doesn't install XMLCalabash and skip the test

@AirQuick : could you please review this pull request and see if it aligns with your pull request in #86? Once you approve this, I will merge both pull requests into master.

@cirulls cirulls requested a review from AirQuick March 6, 2017 21:31
@cirulls cirulls added this to the v0.6.0 milestone Mar 6, 2017
@cirulls cirulls mentioned this pull request Mar 6, 2017
test/xspec.bats Outdated
@@ -193,6 +193,10 @@ teardown() {


@test "executing the Saxon XProc harness generates a report with UTF-8 encoding" {
if [ -z ${XMLCALABASH_VERSION} ]; then
Copy link
Member

Choose a reason for hiding this comment

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

With this line, to run xspec.bats thoroughly, you have to set both XMLCALABASH_VERSION and XMLCALABASH_CP.
Can you consider checking the existence of XMLCALABASH_CP instead of XMLCALABASH_VERSION? That makes it a little bit easier to run xspec.bats on local console without .travis.yml, which I do occasionally.

Copy link
Member Author

Choose a reason for hiding this comment

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

The check in xspec.bats is now based on XMLCALABASH_CP instead of XMLCALABASH_VERSION (which makes more sense because XMLCalabash is properly installed only once XMLCALABASH_CP is set).

Have you got any further changes?

AirQuick added a commit to AirQuick/xspec that referenced this pull request Mar 7, 2017
AirQuick added a commit to AirQuick/xspec that referenced this pull request Mar 7, 2017
Copy link
Member

@AirQuick AirQuick left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good to me.

cirulls pushed a commit that referenced this pull request Mar 7, 2017
* The test for XProc harness for Saxon is executed only when a Saxon 9.7.0.-15 is used

* Expect UTF-8

* Log skip when XProc test was not run

* Align appveyor.yml with #96

* Align xspec-bat.cmd with #96 (should fail)

* Expect UTF-8
@cirulls
Copy link
Member Author

cirulls commented Mar 7, 2017

Thanks for reviewing the pull request and suggest changes. This is good to go and I'll merge into master.

@cirulls cirulls merged commit f4811a5 into master Mar 7, 2017
@cirulls cirulls deleted the feature/86 branch March 7, 2017 09:50
fi

run java -Xmx1024m -cp ${XMLCALABASH_CP} com.xmlcalabash.drivers.Main -isource=xspec-72.xspec xspec-home=file://${TRAVIS_BUILD_DIR}/ -oresult=${TRAVIS_BUILD_DIR}/test/xspec/xspec-72-result.html ${TRAVIS_BUILD_DIR}/src/harnesses/saxon/saxon-xslt-harness.xproc
run java -cp ${SAXON_CP} net.sf.saxon.Query -s:${TRAVIS_BUILD_DIR}/test/xspec/xspec-72-result.html -qs:"declare default element namespace 'http://www.w3.org/1999/xhtml'; /html/head/meta[@http-equiv eq 'Content-Type']/@content = 'text/html; charset=UTF-8'" !method=text
Copy link
Member

@AirQuick AirQuick Mar 7, 2017

Choose a reason for hiding this comment

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

Can you take the run java lines from #86? It corresponds to the initial comment of #86:

  • xspec.bats is also modified a bit. (in order to be more platform independent)
  • '
' is added to XQuery. (in order to avoid conflict with the "Picked up JAVA_TOOL_OPTIONS..." stderr line which is emitted when the JAVA_TOOL_OPTIONS environment variable is defined)

Copy link
Member

Choose a reason for hiding this comment

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

Oops. Already merged to master. I'll make another pull request for this...

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, for once I was too quick ;-) Yes, please make another pull request so that we can trace every change. Thanks.

Copy link
Member

Choose a reason for hiding this comment

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

submitted #98

@AirQuick AirQuick assigned cirulls and unassigned AirQuick and cirulls Mar 7, 2017
AirQuick added a commit to AirQuick/xspec that referenced this pull request Mar 14, 2017
* master:
  Align Travis configuration with AppVeyor in #86 (xspec#96)
  Run XProc test on AppVeyor (#86)
  Highlight the link target (#79)
  Consolidate cleanup tasks in batch script(#73)
AirQuick added a commit to AirQuick/xspec that referenced this pull request Mar 15, 2017
* 'master' of https://github.com/AirQuick/xspec:
  Align Travis configuration with AppVeyor in #86 (xspec#96)
  Run XProc test on AppVeyor (#86)
  Highlight the link target (#79)
  Consolidate cleanup tasks in batch script(#73)
AirQuick added a commit to AirQuick/xspec that referenced this pull request Mar 15, 2017
* 'master' of git://github.com/xspec/xspec:
  add XProc harness for Saxon from v0.4.0 (xspec#108)
  revert to commit f4811a5 as tests are failing on master xspec#97 (xspec#101)
  Revive @Assert and @context.
  Fix indent.
  Get the URLs back, adapted.
  Align Travis configuration with AppVeyor in #86 (xspec#96)
  Run XProc test on AppVeyor (#86)
  Highlight the link target (#79)
  Consolidate cleanup tasks in batch script(#73)
  Revert harnesses to v0.3.0 and add UTF-8 encoding #74 #72 (#76)
  Run tests with Saxon 9.7.0-15 (#77)
  Keep type annotations in $impl:test-items (#63)
AirQuick added a commit to AirQuick/xspec that referenced this pull request Mar 15, 2017
* 'master' of git://github.com/xspec/xspec:
  add XProc harness for Saxon from v0.4.0 (xspec#108)
  revert to commit f4811a5 as tests are failing on master xspec#97 (xspec#101)
  Revive @Assert and @context.
  Fix indent.
  Get the URLs back, adapted.
  Align Travis configuration with AppVeyor in #86 (xspec#96)
  Run XProc test on AppVeyor (#86)
@cirulls cirulls self-assigned this Apr 8, 2017
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.

None yet

2 participants