-
-
Notifications
You must be signed in to change notification settings - Fork 43
Switch to the new templating module, remove shared-resources dependency #626
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
Conversation
|
As is with this PR, loading http://localhost:8080/exist/apps/doc/ returns a 400 error: err:XPTY0004 Invalid cardinality for parameter $field. Expected exactly one, got 0 [at line 24, column 22, source: /db/apps/doc/modules/docbook.xql]
In function:
docbook:load(node(), map(*), xs:string?, xs:string?, xs:string) [-1:-1:/db/apps/doc/modules/docbook.xql]
templates:process-output(element(), map(*), item()*, element(function)) [211:9:/Users/joe/Library/Application Support/org.exist/expathrepo/shared-0.9.1/content/templates.xql]
templates:call-by-introspection(element(), map(*), map(*), function(*)) [189:28:/Users/joe/Library/Application Support/org.exist/expathrepo/shared-0.9.1/content/templates.xql]
templates:call(item(), element(), map(*)) [137:36:/Users/joe/Library/Application Support/org.exist/expathrepo/shared-0.9.1/content/templates.xql]
templates:process(node()*, map(*)) [148:81:/Users/joe/Library/Application Support/org.exist/expathrepo/shared-0.9.1/content/templates.xql]
templates:process(node()*, map(*)) [148:81:/Users/joe/Library/Application Support/org.exist/expathrepo/shared-0.9.1/content/templates.xql]
templates:process(node()*, map(*)) [148:81:/Users/joe/Library/Application Support/org.exist/expathrepo/shared-0.9.1/content/templates.xql]
templates:process(node()*, map(*)) [20:23:/Users/joe/Library/Application Support/org.exist/expathrepo/shared-0.9.1/content/templates.xql]
site:expand-links(node(), map(*), xs:string?) [-1:-1:/Users/joe/Library/Application Support/org.exist/expathrepo/shared-0.9.1/content/siteutils.xql]
templates:process-output(element(), map(*), item()*, element(function)) [210:9:/Users/joe/Library/Application Support/org.exist/expathrepo/templating-1.0.0/content/templates.xqm]
templates:call-by-introspection(element(), map(*), map(*), function(*)) [188:28:/Users/joe/Library/Application Support/org.exist/expathrepo/templating-1.0.0/content/templates.xqm]
templates:call(item(), element(), map(*)) [144:37:/Users/joe/Library/Application Support/org.exist/expathrepo/templating-1.0.0/content/templates.xqm]
templates:process(node()*, map(*)) [147:81:/Users/joe/Library/Application Support/org.exist/expathrepo/templating-1.0.0/content/templates.xqm]
templates:process(node()*, map(*)) [430:17:/Users/joe/Library/Application Support/org.exist/expathrepo/templating-1.0.0/content/templates.xqm]
templates:process-output(element(), map(*), item()*) [229:9:/Users/joe/Library/Application Support/org.exist/expathrepo/templating-1.0.0/content/templates.xqm]
templates:process-output(element(), map(*), item()*, element(function)) [210:9:/Users/joe/Library/Application Support/org.exist/expathrepo/templating-1.0.0/content/templates.xqm]
templates:call-by-introspection(element(), map(*), map(*), function(*)) [188:28:/Users/joe/Library/Application Support/org.exist/expathrepo/templating-1.0.0/content/templates.xqm]
templates:call(item(), element(), map(*)) [136:36:/Users/joe/Library/Application Support/org.exist/expathrepo/templating-1.0.0/content/templates.xqm]
templates:process(node()*, map(*)) [132:51:/Users/joe/Library/Application Support/org.exist/expathrepo/templating-1.0.0/content/templates.xqm]
templates:process(node()*, map(*)) [89:9:/Users/joe/Library/Application Support/org.exist/expathrepo/templating-1.0.0/content/templates.xqm]
templates:apply(node()+, function(*), map(*)?, map(*)?) [29:5:/Users/joe/Library/Application Support/org.exist/expathrepo/templating-1.0.0/content/templates.xqm]This error message points to For some reason, with the new templating library, the function's annotation Looking at I tried changing the function signature for @wolfgangmm I'm stuck. Do you have any pointers on overcoming this issue? Other than this, the app's styling seems to have gone wrong: |
|
We may want to pause on merging this PR until eXist-db/exist#3918 is resolved. The |
line-o
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That all looks good to me.
|
What do we do about the failing tests? From what I understand they cannot be found. |
|
@line-o I should've mentioned that I ran the tests locally, and This is odd because in Chrome, Developer Tools shows that the request to http://localhost:8080/exist/apps/doc/ returns a content type of As to the CI failures, they long predate this PR and are completely opaque to me. As far as I can tell, they should be running |
|
@joewiz can you rebase this? |
|
@duncdrum Sure, I've rebased it. Tests (i.e., |
|
I just checked out the PR and packaged the app. Running Side note: In the job details it states |
|
@line-o this does not look like a config problem with the CI. Dependabot and my PR are working fine, the response from the exist container is not what the test result parser expects. This is now all configured via |
|
Ah! of course! |
|
@joewiz that would totally explain a failure, but the messages are telling a different story. |
|
With #632 merged, I rebased again, and CI tests on this branch appear to still be failing when executing the mocha test that calls the XQsuite test suite: This is strange, because on my local system, {
"testsuite": {
"package": "http://exist-db.org/xquery/documentation/tests",
"timestamp": "2021-06-08T13:00:44.146-04:00",
"tests": "6",
"failures": "0",
"errors": "0",
"pending": "1",
"time": "PT1.419S",
"testcase": [
{
"name": "Listing consistency",
"class": "tests:equal-listing"
},
{
"name": "section-headings",
"class": "tests:missing-id"
},
{
"name": "Pro angular brackets",
"class": "tests:no-ecaped-listings"
},
{
"name": "wellformed xml",
"class": "tests:no-txt-xmls",
"pending": null
},
{
"name": "diagnose listings",
"class": "tests:orphan-listing"
},
{
"name": "ToC rendering",
"class": "tests:toc-inline"
}
]
}
}Why is CI not able to see this? Perhaps the test-runner.xql query isn't running at all? |
|
@duncdrum Interesting. I tried extending That said, templates v1.0.2 is only 9k, is a library module that installs basically instantly, and with my PR the documentation app is faster than before - loading in 230-250ms. (The current stable version loads in 240-260ms.) Is there any way to add some logging to have a better idea of what mocha actually receives when it requests |
|
@duncdrum Thanks for pointing out that you had long ago removed dependencies on shared-resources. I just cleared up a couple of lingering references to it. |
|
The tests still run on node 10 also a possible source of problems. |
|
Oops! Not sure how I managed to close it. |
|
@line-o I rebased and bumped node.version to 14 in my latest commit, but, alas, the error seems to be unchanged - "Cannot read property 'tests' of undefined": |
|
@joewiz since you bumped the node version would you please add the updated package-lock.json to version control. |
|
Why did you choose npm version 6.14.13, is that the recommended version for long term support? |
|
I think it is safe to use npm v7.16.0 (the current stable release). |
|
@line-o I went with 6.4.13 because https://nodejs.org/en/download/ says:
I think I'll wait for progress on #638 before taking further action here - to avoid crossing streams. |
|
Since node/npm issues are being pursued through #638, I removed the node/npm commit and force-pushed the branch. This branch still fails CI checks, though of course they still pass locally. Fingers crossed we can resolve the CI issue through #638. Worst case scenario, so as not to hold up eXist 5.3.0: we can publish a new release of the documentation from master, despite its continued dependency on shared-resources (namely, the templates call to |
5b37ada to
a76bdce
Compare
|
With master reliably passing CI, I rebased this PR against master and hoped this branch would now pass. Alas, it still failed with the same missing
So in my latest commit I added logging to the mocha/xqSuite.js file. Now, (a) the actual error is returned, and (b) the body of eXist's response to the request for Here's what we can now see in the CI logs: This tells me that even though eXist has started (meaning the doc xar MUST be running since in CI, the xar is deposited into the autodeploy directory, and eXist always installs autodeploy xars before it reports "Server has started, listening on"), the request to The identical tests pass with no problem on my local system. Why would a request to Is it possible the app hasn't actually been installed yet even though eXist has started? Has the CI somehow cached some stale assets from before we solved the CI problems with master? |
|
I was also not able to reproduce the issue on my local machine. diff --git a/src/test/mocha/xqSuite.js b/src/test/mocha/xqSuite.js
index 8859130..5b7f286 100644
--- a/src/test/mocha/xqSuite.js
+++ b/src/test/mocha/xqSuite.js
@@ -6,7 +6,7 @@ var client = supertest.agent('http://localhost:8080')
describe('running XQsuite test …', function () {
this.timeout(95000)
this.slow(60000)
- const runner = '/exist/rest/db/apps/doc/modules/test-runner.xql'
+ const runner = '/exist/apps/doc/modules/test-runner.xql'
it('returns 0 errors or failures', function (done) {
clientBut as already established the problem seems to be that the test-runner.xql is missing.
|
|
If we find the above thesis to be true, then the installation process needs to either:
|
|
@joewiz @adamretter @duncdrum Does one of you remember where the example for getting a specific dependency with maven is? |
|
Hmm I had a look around but could not find a maven task nor a GitHub Workflow. No I start to doubt my memory on this. |
|
@line-o The facility was already part of the build, it was still pointing at the old |

In preparation for release of eXist 5.3.0 - see eXist-db/exist#3914.