-
-
Notifications
You must be signed in to change notification settings - Fork 32
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
xqlint error causes "run test" to silently fail #224
xqlint error causes "run test" to silently fail #224
Comments
As noticed by @dizzzz and me, xqlint parsing errors still will prevent eXide from even executing xqsuite tests via the "XQuery > Run as test" menu. I have a very rough workaround in https://github.com/joewiz/eXide/tree/disable-xqlint-for-xqsuite, but I hesitate to even propose this as a PR, since we lose functionality, as I describe in the commit. That said, the branch does allow me to resume use of eXide for performing xqsuite tests that contain syntax that, for some reason, offends xqlint (or rather the ancient fork of it that eXide is stuck on). |
@dizzzz I understand from our conversation in Slack that the https://github.com/joewiz/eXide/tree/disable-xqlint-for-xqsuite branch isn't making the xqlint-for-xqsuite problem go away. Could you please post a link to the test module you're using, or post the module here as an attachment? Then I can test it on my system and try to prevent xqlint from firing on your module? |
store as xquery version "3.1";
module namespace m = "aaaaaaaaaaa";
declare namespace test = "http://exist-db.org/xquery/xqsuite";
declare variable $m:one := map {
"a" : "b",
"c" : "d"
};
declare variable $m:two := map {
"a" : "b",
"c" : "d"
};
declare
%test:assertEquals("this should fail")
function m:testSimple() as xs:string {
"aaaa"
}; |
@dizzzz Your test works for me in eXide, built from my branch. I copied and pasted your query into eXide, saved it as /db/test.xq, and then selected XQuery > Run as test. The query result was: <testsuites>
<testsuite package="aaaaaaaaaaa" timestamp="2019-09-29T18:29:22.679-04:00" tests="1" failures="1" errors="0" pending="0" time="PT0.006S">
<testcase name="testSimple" class="m:testSimple">
<failure message="assertEquals failed." type="failure-error-code-1">this should fail</failure>
<output>aaaa</output>
</testcase>
</testsuite>
</testsuites> I've attached my build of eXide here, since it must be that something is preventing your system from picking up the changes in my branch. eXide-2.4.9.zip - disable-xqlint-for-xqsuite build After downloading, just change zip to xar, and install via Package Manager; then in eXide, shift-click on the reload button to ensure that the browser doesn't load stale copies of eXide.min.js. Then:
|
Another totally valid approach to running xqsuite tests in eXide (or anywhere) is to invoke the xqsuite module manually. All you have to do is write a query like this: xquery version "3.1";
import module namespace test="http://exist-db.org/xquery/xqsuite" at "resource:org/exist/xquery/lib/xqsuite/xqsuite.xql";
test:suite(
inspect:module-functions(xs:anyURI("/db/test.xq"))
) This approach is documented in https://exist-db.org/exist/apps/doc/xqsuite#running-testsuite. |
@ccheraa @plutonik-a While some of the original tests given here do now pass, the issue can still be triggered if you use syntax that xqlint chokes on. For an example, follow the steps in the linked issue above: eXist-db/exist#3417. I'll add the test here for convenience. Steps to reproduce
The Javascript console error shows that, as described in the original post, an xqlint error can cause eXide's "Run as test" to silently fail. Thus, part 1 of the fix to this issue is to apply a necessary band aid for cases when xqlint's parser chokes on a query. Specifically, eXide should catch xqlint errors like this and report the xqlint parsing error to the user. Something like this from the console above should be shown:
It should probably be shown where other eXide errors are shown - in red text in the lower right corner of the editor pane. Judging by the location of the parsing error - at line 19 column 35 of the query - xqlint is tripping on test.xq's valid use of the XQuery 3.1 String Constructor syntax, i.e., Looking through the EBNF in the parser, the version of xqlint being used by eXide doesn't have a definition for the String Constructor. Thus, part 2 of the fix is to update the EBNF to add the String Constructor and update the parser used by eXide. For one such PR, see eXist-db/xqlint#1. For additional context, Note that the xqlint used by eXide is a very old fork of https://github.com/wcandillon/xqlint. That project has since updated its tooling to use more modern Javascript, but in a way that @wolfgangmm found difficult to update eXide to use. But he did use a newer fork of the upstream xqlint repository for his atom and vscode plugins. The xqlint fork used for those plugins is now 38 commits behind the original fork. It doesn't have the needed EBNF additions for the string constructor. But this is all to say that one alternative approach to "part 2" here is to switch from the old fork to the current upstream master. The EBNF additions would still be needed (hopefully, they could simply be copied from the XQuery spec linked above), but at least the tooling for building xqlint would be much newer. |
@marmoure A simpler test xqsuite module to reproduce the same issue: xquery version "3.1";
module namespace t="http://exist-db.org/xquery/test";
declare namespace test="http://exist-db.org/xquery/xqsuite";
declare variable $t:foo := ``[foo]``;
declare
%test:assertEquals("foo")
function t:simple-test() {
$t:foo
}; Just replace the query in the 1st step of my previous post with this. |
@ccheraa I see you mentioned this issue in a comment on a PR submitted to a forked copy of this repository: evolvedbinary#1. Is that PR part of the fix to this issue? If so, should that PR be resubmitted to this repository? |
This is not a fix for this issue, it only displays a comprehensive error message when eXide's and eXist-db's XQuery parsers are not the same version, instead of failing silently. |
As discussed and noted in #529, that PR was an important but partial fix. A full fix to this issue requires updating xqlint to accept valid XQuery 3.1 syntax. |
It turns out updating eXist-db/xqlint to support |
@ccheraa Could you provide a bit more detail please? What commit and what feature is it specifically that eXide relies on in the older https://github.com/eXist-db/xqlint that is not present in a more modern xqlint? |
https://github.com/eXist-db/xqlint has some changes that helps connect it to eXide, which are missing from https://github.com/wcandillon/xqlint |
When running a test with the spec-compliant map constructor syntax, eXide silently fails, with this console error:
Correcting the xqlint parser at https://github.com/wolfgangmm/xqlint/blob/master/lib/XQueryParser.ebnf#L426 should solve this immediate issue, though it's unfortunate that the parsing error isn't reported explicitly to the user in the application.
We should probably test atom-existdb to see if its xqlint fork also shares this issue. https://github.com/eXist-db/atom-existdb/blob/master/package.json#L47 suggests that it's using this fork: https://github.com/eXistSolutions/xqlint/tree/master/lib. I can't actually find any trace of support for the map constructor in this repo...
The text was updated successfully, but these errors were encountered: