-
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
Fix/revert revert reverting #103
base: master
Are you sure you want to change the base?
Conversation
- fix the input port not bound in the last p:error in harness-lib - fix the xspec-home not extracted from parameters on the format step
@fgeorges: Thanks for your pull request. I started reviewing it and have several questions, I hope you can provide us with the information we were not able to find out when we reverted to v0.3.0 for attributes First of all, I listed all the files that have changed. There are lots of changed files and unfortunately the files changed view in GitHub is not very helpful in this case as it doesn't not remove trivial changes like whitespaces and carriage returns. Files that have been added or changed
Files that have not changed or where the change is trivial (e.g. whitespace)
Files deleted
Questions
ReviewThere is lots to review as these are actually two pull requests in one (attributes @AirQuick: would you have time to review one part of this pull request? I'm happy to review the XProc harness part as I remember you previously mentioned you are not familiar with all the tools (I'm not either but I can give it a go with help from Florent). |
(a) Leave it as it is for now. There are still some work to be done on (b) They are XProc pipeline to drive executing XSpec on different processors. Assuming only Saxon is wrong. So they are supposed to be invoked from command line using Calabash (the same way the harness for Saxon is invoked). Each pipeline contains documentation on how it should be invoked, its options, etc. The level of maturity of these harnesses varies from each other. They at least have the merit to exist, instead of having nothing at all for other processors, and give as a minimum one way to use XSpec on these processors. (c) This line explains why the step next line is commented out. The step is supposed to provide extra checks. It is not implemented yet. Not a big deal. (d) There is more than an example to had. It should be documented, tested, etc. There are emails on the XSpec group discussing all this. One of the primary reasons for introducing This is a "beta feature". We want to keep the possibility to change it slightly in a future release based on feedback. (e) I guess so. I don't know who/what that schema is used by/for. Probably best to wait for a "definitive" version of them before adding them. Especially if that schema is used for auto-completion, e.g. in oXygen. (f) In some case (when using a repository with the XAR version of XSpec), some extra lines are output. The test is not about checking that a piece of text is absolutely on a specific line, but that it appears in a way that confirms all is OK. |
Thanks @fgeorges for your replies and for marking the questions. I think I have enough information to review the XProc harness part. I agree that it is good to have something for processors other than Saxon. I'll try to install these processors and run the XProc harnesses. I may ask you some additional questions if I get stuck. If @AirQuick can review the |
In the present situation I'm not joining this activity, sorry... |
@fgeorges: I started reviewing your pull request. As there are several things to review, I'm going to do it little by little so that parts of your pull request can be immediately pushed to I started by reviewing the XProc harness for Saxon. The thing that I missed out when we reverted to v0.3.0 was the I had a look at the XProc test in @test "executing the Saxon XProc harness generates a report with UTF-8 encoding" {
query="declare default element namespace 'http://www.w3.org/1999/xhtml'; /html/head/meta[@http-equiv eq 'Content-Type']/@content = 'text/html; charset=UTF-8'";
if which saxon && which calabash; then
run calabash -isource=xspec-72.xspec -oresult=xspec/xspec-72-result.html ../src/harnesses/saxon/saxon-xslt-harness.xproc
run saxon --xquery -s:xspec/xspec-72-result.html -qs:"$query" !method=text
elif [ -z ${XMLCALABASH_CP} ]; then
skip "test for XProc skipped as XMLCalabash uses a higher version of Saxon";
else
run java -Xmx1024m -cp ${XMLCALABASH_CP} com.xmlcalabash.drivers.Main -isource=xspec-72.xspec -p 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:"$query" !method=text
fi
echo $output
[[ "${lines[0]}" = "true"
|| "${lines[2]}" = "true" ]]
} When this test is run on Travis, the first if condition I modified the test in branch review/103 so that it keeps the changes you made to run the XProc harness and removes the unnecessary conditions which are never executed on Travis. I created branch review/103 which contains your code for the Saxon XProc harness (including Would that be ok for you? |
|
* integrate parts of code review pr #103 for Saxon XProc harness
@fgeorges: thanks for your feedback. I merged this part of the review in #108 and updated the documentation on the wiki. I'll continue by reviewing the other XProc harnesses but it's probably going to be next week as I haven't got enough time during the rest of this week. I'll keep you posted in here. |
@cirulls Any update on this? There start to be conflicts, as it gets desynchronized! |
I started reviewing the XProc harness for BaseX but I'm still trying to run it as I'm not familiar with BaseX. For me it would easier to have one pull request for each component that has changed (e.g. one pull request for each XProc harness + one pull request for each attribute) as this will allow to review and merge the pull requests one by one. If other people could help reviewing, it would be great. |
@cirulls Any update on this one? Conflicts are accumulating... |
Sorry if it is taking longer than expected, I struggle to find enough time to work on several issues and pull requests at the same time. I start reviewing the BaseX part of the pull request and I'm planning to keep working on it during this weekend. Regarding conflicts, it is possible to apply the latest commits merged into branch |
- add XProc harness for BaseX 8.6.4 standalone - add Travis configuration for BaseX - add test for BaseX - add tutorial files for XQuery
Sorry for the delay on this. I reviewed the XProc harness for BaseX standalone and it looks good to me. I extrapolated that part of your pull request and added documentation and tests, please see #136, I added you as reviewer. If this is fine with you, I'll merge the BaseX standalone harness into I tried to execute the XProc harness for BaseX server but I didn't manage to make it run, I am not sure if I'm providing the correct options. This is what I tried:
When I looked at line 110 of basex-server-xquery-harness.xproc, I noticed that Am I not providing the correct parameters? Could you let me know how you run the BaseX server harness? |
* add XProc harness for BaseX standalone (review #103) - add XProc harness for BaseX 8.6.4 standalone - add Travis configuration for BaseX - add test for BaseX - add tutorial files for XQuery * create directory for BaseX * add double square bracket in condition * add semi-colon * add semi-colon in condition for test * test whole output message instead of line
* 'master' of git://github.com/xspec/xspec: Allow Schematron XSLTs to be provided externally (xspec#133) From pull request xspec#129 discussion: suppress warning message and clean up temporary files (xspec#131) add XProc harness for BaseX standalone (review xspec#103) (xspec#136) Add documentation for Schematron support (xspec#129) add Schematron support (xspec#105) add checks for saxon script + test (xspec#124) Run tests with XML Calabash 1.1.16-97 (xspec#123) Escape apostrophe in URI (xspec#119) xspec.bat (Windows) fails when path contains parentheses
* 'master' of git://github.com/xspec/xspec: Add TODO comments for focus/pending tests in #4 (#61) End-to-end test for XSpec itself (#81) Allow Schematron XSLTs to be provided externally (xspec#133) From pull request xspec#129 discussion: suppress warning message and clean up temporary files (xspec#131) add XProc harness for BaseX standalone (review xspec#103) (xspec#136) Add documentation for Schematron support (xspec#129) add Schematron support (xspec#105) add checks for saxon script + test (xspec#124) Run tests with XML Calabash 1.1.16-97 (xspec#123) Escape apostrophe in URI (xspec#119) xspec.bat (Windows) fails when path contains parentheses Do not copy unused namespaces from format utils to output (xspec#91) Stop using functx namespace (xspec#104)
No description provided.