Asciidoctor-Doctest Testing #116

obilodeau · 2016-10-27T03:40:22Z

The beginning of asciidoctor-doctest integration (#92).

At first, I thought this would help us test both slim and jade at the same time but I realize it would need a asciidoctor-doctest.js implementation. Maybe we could add a rake task to generate templates from the javascript/node toolchain and use doctest to do the diff. @Mogztter thoughts on this?

Test samples were generated from current back-end so I think some thought, reviews and more custom examples should be migrated before calling it a go.

Also Travis integration would be really nice.

Tasks

port to using doctest 2.0.0.beta.3
travis-ci integration (rake test)
migrate at least one manual test into a non-embedded test
see what happens with Test options don't work and test comments are stripped on rake generate (and rake test) asciidoctor-doctest#12
review the currently generated HTML and consider dropping the irrelevant stuff

Comments and further contributions welcome!

This is an incomplete pull on the old upstream doctest branch that I adapted based on asciidoctor-doctest's current README.

ggrossetie · 2016-10-27T16:32:15Z

At first, I thought this would help us test both slim and jade at the same time but I realize it would need a asciidoctor-doctest.js implementation. Maybe we could add a rake task to generate templates from the javascript/node toolchain and use doctest to do the diff. @Mogztter thoughts on this?

How did you generate files in output (ie. where is the source files) ? input directory is empty, is this intended ?

obilodeau · 2016-10-27T19:34:00Z

How did you generate files in output (ie. where is the source files) ? input directory is empty, is this intended ?

Yes it is normal. The files in output/ match tests in asciidoctor-doctest's own test set: https://github.com/asciidoctor/asciidoctor-doctest/tree/master/data/examples/asciidoc. Eventually, I'll migrate the manual tests over to input/ which will need new matching results in output/ but I haven't done so yet.

ggrossetie · 2016-10-28T14:11:55Z

Ok so I need to generate one file per example in JavaScript to mimic what Asciidoctor Doctest is doing ?
Can we copy data/examples/asciidoc files over ? It will be easier to read example files.

Currently output directory is what we get now when we are using Slim template or what the expected result should be ? Where should I put the result generated from Jade template ? Should we consider Slim template the reference ?

Thanks
Guillaume

mojavelinux · 2016-11-25T01:39:32Z

The sooner we can get this merged, the better. Then we can all work on improving the examples. wdyt?

ggrossetie · 2017-08-17T17:35:27Z

I'm starting to upgrade Jade/Pug templates to use the new public API with Asciidoctor.js 1.5.6+.

To make sure that I'm not breaking anything, I'm writing a test for each .adoc file in the test/ directory. I'm guessing that maybe we should merge this pull request so I could use doctest in the JavaScrip tests suite ?

@obilodeau What's left to do ?

obilodeau · 2017-08-19T04:53:19Z

I made a custom test working today. See test/input/document.adoc.

The manual tests are not ported to the doctest setup and I'm still trying to workaround the fact that all my tests would need to be in a very large document.adoc.

What would I need to do to test using the javascript toolchain?

obilodeau · 2017-08-19T07:37:49Z

[...] I'm still trying to workaround the fact that all my tests would need to be in a very large document.adoc.

Turned out this was an assumption I made. I am converting the tests to doctest right now (adding metadata). Making some reorganization while I'm at it. But now I need to get some 🛌.

So all test/*.adoc files should have slim renders and be compared when running bundle exec rake test pretty soon.

Hopefully, that rendering will be stable enough between jade and slim so that we can use two different templating engine but only one HTML result set. We'll see soon enough.

In the mean time, anything (instructions) that can help me run rake tests using the javascript toolchain would be appreciated 😄

ggrossetie · 2017-08-19T12:34:54Z

I think it won't be easy to cross compile asciidoctor-doctest to JavaScript because the library has a few dependencies.
Maybe we could mimic the behavior of doctest in JavaScript to generate tests ?

The basic idea is to compare an input with an expected output:

var asciidoctor = require('asciidoctor.js')();
require('asciidoctor-template.js');
require('../build/asciidoctor-revealjs.js');
var assert = require('assert');
var fs = require('fs');

describe('Rendering', function () {

  var tests = [
    {input: './test/block_admonition.adoc', expected: './test/block_admonition_output.html'},
    {input: './test/block_audio.adoc', expected: './test/block_audio_output.html'},
    {input: '...', expected: '...'}
  ];

  tests.forEach(function (test) {
    it('should produce a valid Reveal.js presentation for ' + test.input, function () {
      var html = asciidoctor.convertFile(test.input, {safe: 'safe', backend: 'revealjs', to_file: false});
      assert.equal(html, fs.readFileSync(test.expected, 'utf-8'));
    });
  });
});

obilodeau · 2017-08-19T14:51:43Z

Maybe we could mimic the behavior of doctest in JavaScript to generate tests ?

The problem here is that we won't leverage doctest's power unless we reimplement it the same. Features like showing exactly where the problem is introduced with colorized diffs. Also, the output will probably be different since it will use another HTML beautifier and I'm afraid we won't be able to use the same test output for both slim and jade (ensuring compatibility between the two).

That said, I think this is the only real option we've got for now unless asciidoctor-doctest's maintainer is interested in making it transcompilable.

ggrossetie · 2017-08-19T15:02:40Z

The problem here is that we won't leverage doctest's power unless we reimplement it the same. Features like showing exactly where the problem is introduced with colorized diffs. Also, the output will probably be different since it will use another HTML beautifier and I'm afraid we won't be able to use the same test output for both slim and jade (ensuring compatibility between the two).

That's right, you will have the diff output of the chosen JavaScript framework.
But the output is not that bad (at least with mocha):

* test cases are now in examples/ which should make them easier to discover * rendered tests are in test/output/slim so we can have jade/ at some point * most test cases are not useful right now due to issue: asciidoctor/asciidoctor-doctest#12

obilodeau · 2017-08-19T16:35:43Z

So all test/*.adoc files should have slim renders and be compared when running bundle exec rake test pretty soon.

That's done. I also moved the test cases to examples/ as I find it easier to discover for users. Using the doctest toolchain, nothing would prevent us to have both examples/*.adoc files for complete examples and test/*.adoc for corner-case or regression tests.

Because of asciidoctor/asciidoctor-doctest#12 most tests are useless now.

Next up is travis-ci integration (running rake test on github pushes)

jirutka · 2017-08-19T17:39:14Z

@obilodeau #116 (comment):

it would need a asciidoctor-doctest.js implementation

No, you don’t need. Doctest doesn’t need any deep integration with Asciidoctor, it can be modified to just call external program (asciidoctor.js) that accepts arguments and dumps output to stdout.

@Mogztter #116 (comment):

Ok so I need to generate one file per example in JavaScript to mimic what Asciidoctor Doctest is doing ?

No, one file may contain multiple examples, typically relevant to one particular AST node (e.g. inline_anchor, olist, …). It’s all described here.

@Mogztter #116 (comment):

Can we copy data/examples/asciidoc files over ? It will be easier to read example files.

You can, but really shouldn’t. It’d be just code duplication with all its drawbacks.

@obilodeau #116 (comment):

I'm still trying to workaround the fact that all my tests would need to be in a very large document.adoc.

Why? This is not how it should be used.

@obilodeau #116 (comment):

Hopefully, that rendering will be stable enough between jade and slim so that we can use two different templating engine but only one HTML result set. We'll see soon enough.

Doctest “normalizes” HTML for comparison, so if both templates produce syntactically equal output (formatting, attributes order etc. is irrelevant), then they will be equal.

Why do you need two sets of templates in two different templating engines? Asciidoctor.js is Asciidoctor transpiled to JS using Opal, right? Then why not to transpile even templates, specifically Ruby code generated by the template engine?

You can use asciidoctor-templates-compiler to compile set of Slim templates into single Ruby file – Asciidoctor converter. When you avoid using Slim’s splat attributes (and maybe some other extra extensions), then the resulting code is standalone, i.e. doesn’t depend on Slim or Temple at all. I wrote it for asciidoctor-html5s.

@Mogztter #116 (comment):

I think it won't be easy to cross compile asciidoctor-doctest to JavaScript because the library has a few dependencies.

Why you would do that? That’s not needed at all.

@obilodeau #116 (comment):

That said, I think this is the only real option we've got for now unless asciidoctor-doctest's maintainer is interested in making it transcompilable.

I’m not gonna make doctest transcompilable, because it doesn’t make any sense for me. As I know, Asciidoctor.js runs on node.js, so it should be possible to call it externally as a command, pass it some arguments and gather HTML on stdout, shouldn’t be? That’s all what’s needed for integration with Doctest. It currently does not support this out-of-box, but I’d be happy to implement it.

jirutka · 2017-08-19T17:50:15Z

BTW, please use asciidoctor-doctest 2.0.0.beta.3 or dev branch, not the last stable or master branch. I’ve started almost complete rewrite/refactoring of doctest two years ago and came back to it few weeks ago. The new version is much easier to integrate into project and also more cleanly extensible.

obilodeau · 2017-08-19T17:52:24Z

Thanks @jirutka for the detailed answers.

Why do you need two sets of templates in two different templating engines? Asciidoctor.js is Asciidoctor transpiled to JS using Opal, right? Then why not to transpile even templates, specifically Ruby code generated by the template engine?

You can use asciidoctor-templates-compiler to compile set of Slim templates into single Ruby file – Asciidoctor converter. When you avoid using Slim’s splat attributes (and maybe some other extra extensions), then the resulting code is standalone, i.e. doesn’t depend on Slim or Temple at all. I wrote it for asciidoctor-html5s.

I think this right here is what we should evaluate first.

As I know, Asciidoctor.js runs on node.js, so it should be possible to call it externally as a command, pass it some arguments and gather HTML on stdout, shouldn’t be? That’s all what’s needed for integration with Doctest. It currently does not support this out-of-box, but I’d be happy to implement it.

This would be an acceptable compromise if we can't get compiled templates to work.

@mojavelinux, @Mogztter: thoughts?

BTW, please use asciidoctor-doctest 2.0.0.beta.3 or dev branch, not the last stable or master branch. I’ve started almost complete rewrite of doctest two years ago and came back to it few weeks ago. The new version is much easier to integrate into project and also more cleanly extensible.

Ok, I will update this PR so that we use 2.0.0.beta3.

Edit: I updated task list at the top of the PR to reflect current status.

jirutka · 2017-08-21T15:39:08Z

asciidoctor-revealjs.gemspec

@@ -29,4 +29,6 @@ Gem::Specification.new do |s|
  s.add_runtime_dependency 'thread_safe', '~> 0.3.5'

  s.add_development_dependency 'rake', '~> 10.4.2'
+  s.add_development_dependency 'asciidoctor-doctest', '~> 2.0.0.beta.4'


I’d be better to fix this version (= 2.0.0.beta.4), ’cause it’s a beta and there may come some incompatible changes.

I'm a cybersecurity person and I think it's always better to enable upstream projects to patch their issues but I see your point. I'll pin to that version but I hope you'll keep us in the loop for improvements / security issues 😉

jirutka · 2017-08-21T15:42:59Z

test/output/slim/block_audio.html

+<!-- .basic -->
+<div class="audioblock">
+  <div class="content">
+    <audio controls="" src="ocean_waves.mp3">Your browser does not support the audio tag.</audio>


There should be controls, without ="" (and the same situation with loop and some other attributes). This is problem with nokogiri, it doesn’t not support HTML5 well. :( However, you can manually fix it in generated examples, it can parse it well and assert passes. It’s not a big deal though.

File no longer exists after the refresh

Aaah it's named audio.html now. I fixed it. Commit coming soon.

jirutka · 2017-08-21T15:45:43Z

test/output/slim/block_admonition.html

+  </table>
+</div>
+
+<!-- .basic_multiline -->


This file is outdated, I changed names format in beta.3 to use hyphen instead of underscore.

You can use the following to update them:

sed -Ei '/^/ s/_/-/g' *.html sed -Ei '/^<!-- \.(\w+)$/ s/_/-/g' *.html

However, there are more changes in admonition examples, so better to generate them from scratch

Ah, and also I’ve dropped block_ prefix in beta.3, to be consistent with Asciidoctor (IIRC block_ prefix in node names is a legacy).

jirutka · 2017-08-21T15:54:51Z

test/output/slim/document.html

@@ -0,0 +1,2194 @@
+<!-- .title -->


Document examples should be scoped (using :include: option), look at https://github.com/jirutka/asciidoctor-html5s/blob/master/test/examples/html5/document.html for inspiration. (Just add :include: options and run doctest:generate FORCE=y to update them.

I see now why the options were coming from the output document! Because I have no control on the input ones. Makes sense. Thanks for the pointer!

jirutka · 2017-08-21T15:56:50Z

test/output/slim/inline_anchor.html

+
+<!-- .link-with-target-blank -->
+
+<!-- .link-with-role -->


These empty examples don’t look good… some error in reveal.js converter…?

Hm, it seems that you’ve somehow mixed examples generated using older DocTest version and the latest. It’d be better/easier to remove all output examples generated from reference examples and generate them from scratch.

Just done that

However, the inline_anchor.html template you mentioned just got re-generated the same way.

obilodeau · 2017-08-23T03:53:43Z

@jirutka: I addressed all your comments.

ggrossetie · 2017-08-24T21:18:19Z

Rakefile

+  t.converter_opts = { template_dirs: 'templates/slim' }
+end
+
+# TODO add a rake task to render fully working examples to look at them in a browser


We should maybe create an issue to not forget to do it 😉

Don't you always grep for TODO|FIXME before you do a release? ;)

No, do you ? I grep from time to time but it's not integrated in my development process.
That's why I like to create issues on GitHub so I can keep track of what needs to be done.

Implemented in e5fda00

ggrossetie · 2017-08-25T19:50:08Z

I'm not an expert here but this looks fine to me. Good work 👍

jirutka · 2017-08-25T22:34:48Z

There’s a lot of skipped examples (i.e. missing output examples), is it okay…? Run bundle exec rake doctest:test VERBOSE=yes to see which.

obilodeau · 2017-08-28T03:52:08Z

I think skipped examples are fine for now. We can open a compliance issue about them but I would put them out of scope for what we are trying to achieve here.

obilodeau · 2017-08-28T03:53:37Z

Rakefile

+#        :base_dir => 'examples'
+##        :template_dir => 'templates/slim'
+	system "bundle exec asciidoctor-revealjs data-background-oldstyle.adoc", :chdir => 'examples'
+end


@Mogztter: I didn't manage to make convert_file work. base_dir was not honored. Any ideas?

I'm guessing that you may have to configure the safe mode.

The following seems to work:

Asciidoctor.convert_file 'data-background-newstyle.adoc', :safe => 'safe', :backend => 'revealjs', :base_dir => 'examples'

Still not working for me:

$ bundle exec rake render rake aborted! Errno::ENOENT: No such file or directory @ rb_sysopen - data-background-newstyle.adoc /home/olivier/src/asciidoc/asciidoctor-reveal.js/.bundle/gems/ruby/2.4.0/gems/asciidoctor-1.5.6.1/lib/asciidoctor.rb:1575:in `initialize' /home/olivier/src/asciidoc/asciidoctor-reveal.js/.bundle/gems/ruby/2.4.0/gems/asciidoctor-1.5.6.1/lib/asciidoctor.rb:1575:in `open' /home/olivier/src/asciidoc/asciidoctor-reveal.js/.bundle/gems/ruby/2.4.0/gems/asciidoctor-1.5.6.1/lib/asciidoctor.rb:1575:in `convert_file' /home/olivier/src/asciidoc/asciidoctor-reveal.js/Rakefile:19:in `block in <top (required)>' /usr/bin/bundle:22:in `load' /usr/bin/bundle:22:in `<main>' Tasks: TOP => render (See full trace by running task with --trace)

Should I change cwd with Rake? Anyway, I want the output to be in examples/ and gitignore examples/reveal.js/.

Uh oh I see... I didn't run the right Rake task 😳
convert_file method does not use the base_dir attribute so you'll have to use the full path (see asciidoctor.rb#convert_file)

What about:

Asciidoctor.convert_file 'examples/data-background-newstyle.adoc', :safe => 'safe', :backend => 'revealjs', :base_dir => 'examples'

to_dir should be relative to base_dir so the document should be rendered in the examples/ directory.

If you want the document to be rendered in examples/reveal.js/, I think you need to define :to_dir => 'reveal.js'.

(cherry picked from commit 1dfc983)

obilodeau · 2017-08-29T03:09:55Z

@jirutka I filed #134 for the skipped tests. I'm merging this now.

Thanks everyone for your patience and help through this! It should help development and avoid regressions a lot! 😄

jirutka · 2017-08-29T14:14:53Z

Please next time rebase PR before merging, i.e. remove fixup commits and other noise to get more clean repo history.

obilodeau added 4 commits October 26, 2016 23:30

Started working on doctest.

196bc75

This is an incomplete pull on the old upstream doctest branch that I adapted based on asciidoctor-doctest's current README.

Basic tests are now running with rake with or w/o bundler

da4cf4c

Removed wrong ruby code in unused ToC template (for reveal.js' context)

0df10d9

asciidoctor-doctest integration with generated test corpus

e46e152

This was referenced Oct 27, 2016

Implement asciidoctor-doctest testing #92

Closed

Instructions unclear asciidoctor/asciidoctor-doctest#11

Closed

Avoided double testing, inline now works, cleanup, etc.

d4ea5a2

obilodeau added 2 commits August 18, 2017 18:50

Custom tests PoC

c0fb3d3

Merge branch 'master' into doctest

5ce0932

obilodeau mentioned this pull request Aug 19, 2017

Test options don't work and test comments are stripped on rake generate (and rake test) asciidoctor/asciidoctor-doctest#12

Closed

obilodeau self-assigned this Aug 19, 2017

obilodeau added this to the 1.1.0 milestone Aug 19, 2017

obilodeau mentioned this pull request Aug 19, 2017

Have the ability to use asciidoctor-doctest from the JavaScript Asciidoctor ecosystem asciidoctor/asciidoctor-doctest#13

Open

obilodeau added 2 commits August 19, 2017 12:21

documentation updates

dfd5998

obilodeau mentioned this pull request Aug 19, 2017

Jade templates are behind slim templates #63

Closed

12 tasks

jirutka reviewed Aug 21, 2017

View reviewed changes

obilodeau added 4 commits August 22, 2017 23:42

gitignore: can't use reveal.js at this location so moved in examples

b3fb6cf

Re-generated doctest files and adapted a few of them

84baeb7

doc: hacking document updates based on review comments

aaa015b

gemspec: pinned doctest version based on review comments

cf8d709

obilodeau mentioned this pull request Aug 24, 2017

Compiling slim templates into pure ruby (simplifies Javascript maintenance) #131

Merged

12 tasks

ggrossetie reviewed Aug 24, 2017

View reviewed changes

ggrossetie approved these changes Aug 25, 2017

View reviewed changes

obilodeau added 2 commits August 27, 2017 23:29

Removed no longer used directory

c87a05e

Failed attempt at a Rake task to render all examples slides

715d57f

obilodeau commented Aug 28, 2017

View reviewed changes

obilodeau mentioned this pull request Aug 28, 2017

Asciidoctor 1.5.6 introduced a rendering regression for data-background new-style images #132

Closed

obilodeau added 5 commits August 28, 2017 20:02

Proper test results for data-background-newstyle and others

94ca263

.gitignore: ignoring rendered examples

4ddfba8

Added a rake task to convert all examples into full working HTML

e5fda00

Pinned asciidoctor version requirement to 1.5.4 (see asciidoctor#132)

3eaf6af

(cherry picked from commit 1dfc983)

Travis: Added examples documents generation to travis script

51f8472

obilodeau mentioned this pull request Aug 29, 2017

Fix skipped doctest tests #134

Open

obilodeau merged commit d5f3dce into asciidoctor:master Aug 29, 2017

obilodeau deleted the doctest branch August 29, 2017 03:11

Asciidoctor-Doctest Testing #116

Asciidoctor-Doctest Testing #116

Conversation

obilodeau commented Oct 27, 2016 • edited

ggrossetie commented Oct 27, 2016

obilodeau commented Oct 27, 2016

ggrossetie commented Oct 28, 2016

mojavelinux commented Nov 25, 2016

ggrossetie commented Aug 17, 2017

obilodeau commented Aug 19, 2017

obilodeau commented Aug 19, 2017

ggrossetie commented Aug 19, 2017

obilodeau commented Aug 19, 2017

ggrossetie commented Aug 19, 2017

obilodeau commented Aug 19, 2017

jirutka commented Aug 19, 2017 • edited

jirutka commented Aug 19, 2017 • edited

obilodeau commented Aug 19, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jirutka Aug 21, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jirutka Aug 21, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jirutka Aug 21, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

obilodeau commented Aug 23, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ggrossetie commented Aug 25, 2017

jirutka commented Aug 25, 2017

obilodeau commented Aug 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ggrossetie Aug 28, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

obilodeau commented Aug 29, 2017

jirutka commented Aug 29, 2017

obilodeau commented Oct 27, 2016 •

edited

jirutka commented Aug 19, 2017 •

edited

jirutka commented Aug 19, 2017 •

edited

obilodeau commented Aug 19, 2017 •

edited

jirutka Aug 21, 2017 •

edited

jirutka Aug 21, 2017 •

edited

jirutka Aug 21, 2017 •

edited

ggrossetie Aug 28, 2017 •

edited