Adopt generator-exist, add tests #15

joewiz · 2022-02-05T23:28:23Z

Use generator-exist for scaffolding and integrated testing and CI.

Use templating's gulp integration for building/deploying/test running.

use generator-exist library template, so the package is now a library, not an application
adopt the community convention for XQuery file extensions, using .xqm for library modules and using .xq for main modules
remove controller.xql and parse.xql - since this is now a library package
make minimal changes to pass generator-exist's npm tests
increment version to 1.0.0 given the change from application to library package and the major under-the-hood changes
borrow templating package's gulp-based build/deploy/test
add XQSuite tests based on implicit tests in test.md

Closes #13.

- generator-exist provides common scaffolding and integrates practices for testing and CI - use generator-exist library template, so the package is now a library, not an application - adopt the community convention for XQuery file extensions, using .xqm for library modules and using .xq for main modules - remove controller.xql and parse.xql - since this is now a library package - make minimal changes to pass generator-exist's npm tests (full test suite based on test.md to come) - increment version to 1.0.0 given the change from application to library package and the major under-the-hood changes

The first few examples from test.md - paragraph delimiters - inline-code - inline-code-backticks Created by comparing test.md to HTML produced by the library, e.g., view-source:https://exist-db.org/exist/apps/markdown/test.md

joewiz · 2022-02-06T00:18:51Z

Drat, I just realized I was on an old checkout of the generator-exist "master" branch, whereas development has moved to "main." This explains why some things I was expecting weren't present in the generated package. I will regenerate the package.

- adapted from https://github.com/eXist-db/templating - note that gulp-exist "^4.0.0" (which downloaded 4.3.0) produced errors during `npm test` (reproducible in templating), so I limited it to "4.0.0" until we can get the current gulp-exist working again. See eXist-db/templating#11.

joewiz · 2022-02-06T05:30:51Z

It turns out there weren't many changes in generator-exist main branch.

I was really after the functions I liked from the templating repo - where I could invoke npm test and the xar would be built and deployed before the xqsuite tests were run. It seems that generator-exist still requires you to install the xar yourself before performing tests each times. It was easy enough to copy and paste the necessary gulp invocations from templating. But I was stuck for hours on an error - which I finally realized was a problem in gulp-exist and reported as eXist-db/templating#11. My workaround was to require gulp-exist 4.0.0 instead of letting it install the latest (4.3.0), since the latest caused the error.

So now npm test builds the exist-markdown package, installs it in eXist, and runs the xqsuite tests. Here's the console output:

 % npm test

> markdown@1.0.0 test
> gulp install:all && mocha test/mocha --recursive --exit && mocha test/xqs/*.js

[00:28:00] Using gulpfile ~/workspace/exist-markdown/gulpfile.js
[00:28:00] Starting 'install:all'...
[00:28:00] Starting 'cleanAll'...
[00:28:00] Finished 'cleanAll' after 3.67 ms
[00:28:00] Starting 'templates'...
[00:28:00] Finished 'templates' after 29 ms
[00:28:00] Starting 'copyStatic'...
[00:28:00] Finished 'copyStatic' after 8.35 ms
[00:28:00] Starting 'xar'...
[00:28:00] Finished 'xar' after 21 ms
[00:28:00] Starting 'installLibraryXar'...
[00:28:00] Uploading markdown-1.0.0.xar (9636 bytes)
[00:28:00] Install http://exist-db.org/apps/markdown from markdown-1.0.0.xar
[00:28:00] Application was updated
[00:28:00] Finished 'installLibraryXar' after 568 ms
[00:28:00] Finished 'install:all' after 632 ms


  file system checks
    markup files are well-formed
      ✓ *.html is xhtml
      ✓ *.xml
      ✓ *.xconf
      ✓ *.odd
    Consistent data in aux files
      ✓ Readme should have latest meta-data


  5 passing (32ms)



  0 passing (1ms)



  Xqsuite tests for http://exist-db.org/xquery/markdown/tests
    ✓ Test: Format inline code snippets with a pair of backticks
    ✓ Test: Use two backticks to allow one backtick inside
    ✓ Test: Paragraphs are separated from following blocks by a blank line
    ✓ Test: one-is-one


  4 passing (4ms)

I recall the maven-based apps like semver.xq also enjoy this auto-deploy-during-test feature. I nearly jumped ship for that approach before I discovered the workaround to get gulp-exist working again. Either approach would be fine.

I think I added all the necessary tests. The inline HTML doesn't transform the <mark> element, and others, such as htmlblocks-with-markdown, output too many <body> elements.

Added `%test:pending` for failing tests.

In `declare %annotation function`, the annotations are indented but not the `declare` or `function` keywords. This allows the function body to be indented only one level, rather than 2. Also, applied indenting to expected results for readability.

and delete 'HTML blocks' test which was really a code block test

Implicitly demonstrated with test.md and the operations of the library.

See eXist-db/templating#12.

joewiz · 2022-02-08T04:53:53Z

With @lguariento's kind help and hard work, we now have a test suite containing 16 tests based on the implicit tests in the test.md file at the root of this repository. Of the 16 tests, 13 pass, and 3 are marked pending, since the library does not return the expected output. I will turn each pending test into a new issue, in the hope of closing them and laying a firm foundation for further development of this library.

Here is the XQSuite test report:

<testsuite package="http://exist-db.org/xquery/markdown/tests"
    timestamp="2022-02-07T23:45:51.978-05:00" tests="16" failures="0" errors="0" pending="3"
    time="PT0.023S">
    <testcase name="Atx-style headers and hierarchically nested sections"
        class="tests:atx-style-headers-and-nested-sections"/>
    <testcase name="Code Blocks" class="tests:code-blocks">
        <pending>The raw code block is piped into a p instead of a pre/@data-language=xquery structure</pending>
    </testcase>
    <testcase name="HTML block containing markdown" class="tests:html-block-containing-markdown">
        <pending>Extra body elements are inserted into the divs; div structure is mangled</pending>
    </testcase>
    <testcase name="Images" class="tests:images"/>
    <testcase name="Format inline code snippets with a pair of backticks" class="tests:inline-code"/>
    <testcase name="Use two backticks to allow one backtick inside" class="tests:inline-code-escape-backticks"/>
    <testcase name="Inline HTML" class="tests:inline-html">
        <pending>The mark element is not rendered</pending>
    </testcase>
    <testcase name="Labels" class="tests:labels"/>
    <testcase name="Links" class="tests:links"/>
    <testcase name="Nested list" class="tests:nested-list"/>
    <testcase name="Paragraphs are separated from following blocks by a blank line" class="tests:paragraph-delimiters"/>
    <testcase name="Quotes" class="tests:quotes"/>
    <testcase name="Simple lists" class="tests:simple-list"/>
    <testcase name="Table" class="tests:table"/>
    <testcase name="Task list" class="tests:task-list"/>
    <testcase name="TEI output" class="tests:tei-output"/>
</testsuite>

Thanks to @duncdrum for generator-exist, which provided the new scaffolding for this package and the many affordances for test-driven development, including GitHub Actions support.

Thanks also to @wolfgangmm and @line-o for providing a model of a full build/deploy/test harness in the templating app.

joewiz · 2022-02-08T05:03:58Z

p.s. I'd still gladly welcome contributions of an XQuery wrapper for commonmark-java - along the lines of Andy Bunce's https://github.com/expkg-zone58/ex-markdown XQuery wrapper for flexmark-java, for BaseX. Commonmark has 700 tests and a spec that covers GFM. This is a fun project and all, but we'll be lucky to fix these 3 pending tests...

I'd been reviewing #14 when I created the branch. The change doesn't affect the current test suite, but we'll need tests in that PR before we merge it in to prevent future regressions.

line-o · 2022-02-09T09:17:50Z

Awesome work @joewiz and @lguariento! 🎉

I am happy to merge this. Especially making this a library package really brings this project forward.
A few enhancements we should consider now or later:

drop .existdb.json and putting all necessary package metadata in a property app, exist or xar in package.json
npm packages are implicitly allowed to add their custom properties package.json but have to take care themselves not to clash with names used by npm. I have use app for other projects.
add npm script to install the library (without the test application)
I usually use npm start for that
optimise GithubActions
- use prebuilt docker images (gives cacheing of images for free, faster test preparation)
- also test on docker image tag 5.3.0 (to ensure backwards compatibility for future versions of this lib)
- as npm test already calls gulp install:all copying the XAR into auto deploy of the docker container is superfluous
adopt readOptionsFromEnv to allow all npm and gulp scripts to target different existdb instances

I am eager to hear your feedback on this and happy to help.
A quite complete setup with all of the above can be found in a PR to roaster which is not yet merged.
I would discuss all pros and cons with you first.

duncdrum · 2022-02-09T10:05:16Z

@line-o i m not sure if copying is superflous, since markdown comes as part of the autodeployed packages OOTB iirc. To achieve maximum speedup of tests, i would suggest to host a local volume with just the markdown xar inside into the container's autodeploy.

line-o · 2022-02-09T10:07:03Z

To my knowledge Markdown was dropped from autodeploy since 5.3.0

line-o · 2022-02-09T10:08:08Z

And testing if the package can be installed is therefore useful information

line-o · 2022-02-09T10:08:48Z

Especially since autodeploy does not install dependencies

line-o · 2022-02-09T10:12:39Z

I just checked @duncdrum the markdown package is no longer bundled with exist.

duncdrum · 2022-02-09T10:36:32Z

Ah good if it doesn't ship by default anymore, then there shouldn't be a naming confilct. Installing from autodeploy will install dependencies. Using a volume will theefoe test the install just fine, the thing it does is save us around 30s on every run for exist to install dashboard, monex, … before it becomes responsive

line-o · 2022-02-09T10:43:05Z

OK, so the 30s configuration ramp-up that existdb takes on the very first run is definitely something we should address for CI in general not only this package.

A few possible solutions come to my mind

provide a docker build with an empty auto deploy (that could have tons of applications even outside of CI contexts)
create a shareable GitHub action that pull an exist image runs it once, then snapshots that state and caches it for subsequent uses

line-o

As I said I am happy to merge this as is. Any of the comments I made are just enhancements that do not block this. We can also wait for them to be incorporated into this PR if we choose so.

line-o · 2022-02-09T11:02:54Z

Installing from autodeploy will install dependencies.

@duncdrum how would that be possible to succeed for instances that start without access to the internet?
I am convinced depencies are not installed - if they are missing - for packages added to autodeploy.
That is also why the order of installation matters when adding interdependent packages to autodeploy.

duncdrum · 2022-02-09T11:26:20Z

Install via autodeploy checks to see if any declared dependency is present in the db. So the hack is to make sure that the dependency gets processed first. Nevertheless the check is performed, and installation succeed / fails based on the check.

and yes happy for merge to proceed

line-o · 2022-02-09T12:17:12Z

Install via autodeploy checks to see if any declared dependency is present in the db. So the hack is to make sure that the dependency gets processed first. Nevertheless the check is performed, and installation succeed / fails based on the check.

Yes, correct. That also means no dependencies will be installed, right?

joewiz · 2022-02-13T21:37:17Z

@line-o @duncdrum Thank you for your reviews and discussion. As this PR does what it set out to accomplish, I would be grateful if it could be merged as is and for additional ideas to be pursued in their own follow-on issues and PRs.

joewiz · 2022-02-13T21:49:59Z

@duncdrum Thank you!

@lguariento @line-o @duncdrum This PR is included in https://github.com/eXist-db/exist-markdown/releases/tag/v1.0.0, also available at https://exist-db.org/exist/apps/public-repo/packages/markdown.html?eXist-db-min-version=5.3.0.

joewiz added 2 commits February 5, 2022 18:25

Adapt test.md to XQSuite

4e03938

The first few examples from test.md - paragraph delimiters - inline-code - inline-code-backticks Created by comparing test.md to HTML produced by the library, e.g., view-source:https://exist-db.org/exist/apps/markdown/test.md

joewiz mentioned this pull request Feb 6, 2022

Code blocks properly identified #14

Open

joewiz added 2 commits February 5, 2022 19:34

Regenerate with generator-exist main branch

fe09373

joewiz force-pushed the go-yo branch from 2793bbf to 99ff276 Compare February 6, 2022 05:23

Remove unneeded file

a5f01e3

lguariento marked this pull request as ready for review February 6, 2022 12:14

joewiz marked this pull request as draft February 6, 2022 22:05

joewiz and others added 4 commits February 7, 2022 11:04

Minor textual fixes

1e6ca05

Update test-suite.xqm

1a468cb

I think I added all the necessary tests. The inline HTML doesn't transform the <mark> element, and others, such as htmlblocks-with-markdown, output too many <body> elements.

Update test-suite.xqm

b0d1421

Added `%test:pending` for failing tests.

Place tests in order of test.md

b2a0e01

lguariento approved these changes Feb 7, 2022

View reviewed changes

joewiz added 14 commits February 7, 2022 19:14

Reformat function signatures

645dc51

In `declare %annotation function`, the annotations are indented but not the `declare` or `function` keywords. This allows the function body to be indented only one level, rather than 2. Also, applied indenting to expected results for readability.

Add missing code block test

24d7b9e

and delete 'HTML blocks' test which was really a code block test

Align test names & functions with test.md

dc0b858

Fix tests involving reference-style links

78ffefb

Supply expected result for HTML containing md

740d89b

Fix whitespace in expected results

8b0c7d1

Add h2 section headings in comments

0ce5068

Remove boilerplate test

d481c70

Add test for headers & section nesting

4ff9e6d

Implicitly demonstrated with test.md and the operations of the library.

Align test name with test.md

f7af6e1

Add comment about source of tests

4bfaa59

Port gulp-exist fix from templating

d78223a

See eXist-db/templating#12.

Add test for TEI output

c00f03d

Remove link to ?mode=tei since now only a library

2921a3a

joewiz added 2 commits February 7, 2022 23:45

Markdown is capitalized

c9dc495

Note breaking change in release notes for 1.0.0

0495a76

joewiz marked this pull request as ready for review February 8, 2022 04:54

joewiz added 2 commits February 8, 2022 00:06

Markdown is capitalized

e1ac0d4

Revert inadvertent change

173e01f

I'd been reviewing #14 when I created the branch. The change doesn't affect the current test suite, but we'll need tests in that PR before we merge it in to prevent future regressions.

lguariento approved these changes Feb 8, 2022

View reviewed changes

joewiz requested a review from a team February 8, 2022 19:22

line-o approved these changes Feb 9, 2022

View reviewed changes

line-o added enhancement question labels Feb 12, 2022

duncdrum merged commit d076379 into master Feb 13, 2022

joewiz mentioned this pull request Feb 13, 2022

improve tests; npm and gulp tasks eeditiones/roaster#30

Merged

joewiz deleted the go-yo branch February 13, 2022 21:41

line-o mentioned this pull request Feb 13, 2022

further enhancements #16

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adopt generator-exist, add tests #15

Adopt generator-exist, add tests #15

joewiz commented Feb 5, 2022 •

edited

Loading

joewiz commented Feb 6, 2022

joewiz commented Feb 6, 2022

joewiz commented Feb 8, 2022 •

edited

Loading

joewiz commented Feb 8, 2022 •

edited

Loading

line-o commented Feb 9, 2022 •

edited

Loading

duncdrum commented Feb 9, 2022

line-o commented Feb 9, 2022

line-o commented Feb 9, 2022

line-o commented Feb 9, 2022

line-o commented Feb 9, 2022

duncdrum commented Feb 9, 2022

line-o commented Feb 9, 2022

line-o left a comment

line-o commented Feb 9, 2022 •

edited

Loading

duncdrum commented Feb 9, 2022 •

edited

Loading

line-o commented Feb 9, 2022 •

edited

Loading

joewiz commented Feb 13, 2022 •

edited

Loading

joewiz commented Feb 13, 2022

Adopt generator-exist, add tests #15

Adopt generator-exist, add tests #15

Conversation

joewiz commented Feb 5, 2022 • edited Loading

joewiz commented Feb 6, 2022

joewiz commented Feb 6, 2022

joewiz commented Feb 8, 2022 • edited Loading

joewiz commented Feb 8, 2022 • edited Loading

line-o commented Feb 9, 2022 • edited Loading

duncdrum commented Feb 9, 2022

line-o commented Feb 9, 2022

line-o commented Feb 9, 2022

line-o commented Feb 9, 2022

line-o commented Feb 9, 2022

duncdrum commented Feb 9, 2022

line-o commented Feb 9, 2022

line-o left a comment

Choose a reason for hiding this comment

line-o commented Feb 9, 2022 • edited Loading

duncdrum commented Feb 9, 2022 • edited Loading

line-o commented Feb 9, 2022 • edited Loading

joewiz commented Feb 13, 2022 • edited Loading

joewiz commented Feb 13, 2022

joewiz commented Feb 5, 2022 •

edited

Loading

joewiz commented Feb 8, 2022 •

edited

Loading

joewiz commented Feb 8, 2022 •

edited

Loading

line-o commented Feb 9, 2022 •

edited

Loading

line-o commented Feb 9, 2022 •

edited

Loading

duncdrum commented Feb 9, 2022 •

edited

Loading

line-o commented Feb 9, 2022 •

edited

Loading

joewiz commented Feb 13, 2022 •

edited

Loading