Fix CommonMark test #423

PhrozenByte · 2016-09-05T13:24:48Z

I was wondering what CommonMark examples are still violated by Parsedown, but the current test is broken (its regex matching the CommonMark examples doesn't work correctly). That is why I've developed a working one.

Parsedown currently passes 287 of 616 examples (i.e. 329 failures). Here's a complete phpunit run: http://pastebin.com/uLaJpBWf

FFF.FFFFF........FF.....F.F..F...F....FF....FF..F....FFF.F..FFF  63 / 616 ( 10%)
....FFFF....F..FFF.FFFFFFFFFFF.FFF.FFFFFFFFFFFFFF.F.FFFFFFFFF.. 126 / 616 ( 20%)
FF.FFFF...FF...F.FFFFFF.F.F.FF.FFFF.F.F.F.FF..FFF.FF......FF... 189 / 616 ( 30%)
.F..FFFFF...F...F..F..FFFF.F.....FFFFFF.FFFFF.F.FFFFFFF.FFFFF.. 252 / 616 ( 40%)
.FFFF.FFFF.F.FF.FFFFFFFFFFF.FF.F.FFF.F.FFFFFFFFFFF.FFFFF.F...FF 315 / 616 ( 51%)
..FFF..FF..FFF...FFF.F...FFFF.F.F..F..FF..FFFFFFF.FF....FF.FFF. 378 / 616 ( 61%)
...FFF.....FF.....FFF...............F..FF.......F..FF......F... 441 / 616 ( 71%)
FFFFFFFFFFFF..FF..FFF.FFFF.FFFFF.FF....F..FFFF..FFF..F..FFF.FFF 504 / 616 ( 81%)
.FFFFF.FFF.......F...F...F.....FFFFF.FF....F.F.FF.......FF.F.F. 567 / 616 ( 92%)
.FF.FF....FF.F..FF.....FFF..F.F..F.F.....F.......

FAILURES!
Tests: 616, Assertions: 616, Failures: 329.

One of the main reasons for that many failures is that Parsedown omits whitespaces where whitespaces shouldn't be omitted according to the CommonMark specs. However, many of these whitespaces don't matter when the page is viewed by a browser. That's the reason why I've created the test/CommonMarkTestWeak.php: This test cleans up the HTML markup before comparison, so examples which would normally fail due to actually invisible differences (e.g. superfluous whitespaces), don't fail. However, cleanup relies on block element detection. The detection doesn't work correctly when a element's display CSS property is manipulated. According to that this test should be only a interim solution on Parsedown's way to full CommonMark compatibility.

Parsedown currently passes 338 of 616 examples (i.e. 278 failures) when less aggressively compared. Here's a complete phpunit run: http://pastebin.com/CRNbXSdq

FFF.F.F...........F.....F.F..F...F.....F....FF..F.....FF....FFF  63 / 616 ( 10%)
....F.FF........FF..........FF.FF..FFF...F.F...FF...FFFFFFFFF.. 126 / 616 ( 20%)
FF.FFFF...FF...F.FFFF.F.F.F..F.FFFF.F.F.F.FF..F.F.FF........... 189 / 616 ( 30%)
....FFFFF...F...F..F..F..F.F......FFFFF.FF.FF.F.FFF...F.FFF.F.. 252 / 616 ( 40%)
.FFFF.FFFF.F.FF.FFFFFFF.F.F....F.FFF.F...FFFFFFFFF.FFFFF......F 315 / 616 ( 51%)
..FFF..FF..FFF...FFF.F...FFFF.F.F..F..FF..FFFFFFF.FF....FF.FFF. 378 / 616 ( 61%)
...FFF.....FF.....FFF...............F..FF.......F..FF......F... 441 / 616 ( 71%)
FFFFFFFFFFFF..FF..FFF.FFFF.FFFFF.FF....F..FFFF..FFF..F..FFF.FFF 504 / 616 ( 81%)
.FFFFF.FFF.......F...F...F.....FFFFF.FF....F.F.FF.......FF.F.F. 567 / 616 ( 92%)
.FF.FF....FF.F..FF.....FFF..F.F..F.F.............

FAILURES!
Tests: 616, Assertions: 616, Failures: 278.

Please note that fulfilling all CommonMark spec examples doesn't make Parsedown fully compatible to CommonMark. There are some more things to consider. However, making the examples work is a good starting point.

Please also note that Parsedown not only fails examples which represent "cases that are quite uncommon" (according to the README.md) - some of the failed tests are quite important (e.g. hard line breaks, #334).

erusev · 2016-09-05T14:26:49Z

hard line breaks

Do you mean back slash line breaks, because these are invented by CommonMark and don't exist in the original Markdown syntax.

PhrozenByte · 2016-09-05T14:58:21Z

Yes, exactly.

276) CommonMarkTestWeak::testExample with data set #597 ('Hard line breaks', 'foo\\
baz', '<p>foo<br />
baz</p>')
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'<p>foo<br />
+'<p>foo\
 baz</p>'

However, I have to be honest: I didn't read all failing examples, just a bunch of them. I was focusing on making the tests work 😄

erusev · 2016-09-05T18:22:57Z

Thanks, I've been very busy preparing the next major release of @careteditor, but as soon as I have some time, I'll look into it.

This can be used as a template for Parsedown extensions (like Parsedown Extra) to reuse Parsedown's original tests. You can either extend tests (like ParsedownExtraTest) or reuse them unchanged (like CommonMarkTest). Parsedown extensions simply have to implement their own TestParsedown class (test/TestParsedown.php) and all original tests will run with a instance of this class, rather than a instance of the original Parsedown class. This PR is a follow-up to erusev/parsedown#423, i.e. you must merge erusev/parsedown#423 first, otherwise this doesn't work.

This allows Parsedown extensions (like Parsedown Extra) to reuse existing Parsedown tests. See erusev/parsedown-extra#96 for details.

PhrozenByte · 2016-09-05T19:17:59Z

Ok, great, thanks @erusev 😃

I've now pushed commit 33a23fb to this PR what refactors Parsedown's PHPUnit bootstrap. The goal of this change is to make PHPUnit of erusev/parsedown-extra work again (see erusev/parsedown-extra#96 for details). I wasn't sure whether to open a new PR for this or to add it to this PR, especially because this change is necessary to make the original changes of this PR work with 33a23fb (kind of a dependency hell). If you want me to open another PR for 33a23fb just tell me. 😃

edit: Yet another commit, 73dbe2f. No functional changes compared to 33a23fb, it simply removes the bootstrap script and uses just plain composer.

erusev · 2016-09-05T19:58:24Z

Thanks, hopefully, I'll have a chance to review it soon enough.

PhrozenByte · 2016-10-09T11:03:14Z

btw: It would seem useful to add the CommonMarkTest.php (or CommonMarkTestWeak.php) in a non-breaking way (i.e. || true) to Travis, e.g. with

script:
  - phpunit test/CommonMarkTest.php || true

This way it gets very easy to check whether a PR improves CommonMark compliance or not. Should I add this to the PR @erusev?

erusev · 2016-10-09T11:47:22Z

Sure, it'd take me some time to review the whole thing, though.

PhrozenByte · 2016-10-09T11:52:03Z

Sure, fixing the failing tests definitely is a quite big thing, however, IMHO it is useful to merge this before actually starting to improve CommonMark compliance.

Failing tests don't break builds on purpose, Parsedown doesn't fully comply with the CommonMark specs at the moment. We should switch to test/CommonMarkTest.php later, see erusev#423 for details.

Failing tests don't break builds on purpose, Parsedown Extra doesn't fully comply with the CommonMark specs at the moment. We should switch to test/CommonMarkTest.php of erusev/parsedown later, see erusev/parsedown#423 for details.

erusev · 2016-10-09T12:42:53Z

Sure, hopefully, I'll have some time to look into it in more depth later this month.

Add parameter `$id` to CommonMark tests

CommonMark spec example [erusev#170](http://spec.commonmark.org/0.26/#example-170) has a empty HTML result.

Conflicts: .travis.yml test/CommonMarkTest.php test/ParsedownTest.php test/bootstrap.php

PhrozenByte · 2017-11-14T21:18:02Z

Since there was some progress related to Parsedown's build and test process and Travis lately (#539), I've solved the merge conflicts and would like to express my hope to merge this @erusev.

The changes look bigger than they are, it's basically just a bugfix, but is nevertheless a huge improvement towards Parsedown's build and test process. Parsedown's CommonMarkTest is broken at the moment due to a regex not taking some special cases in the CommonMark spec file into account.

erusev · 2017-11-14T22:12:31Z

@PhrozenByte I think this would make more sense for Parsedown 2.0 which should be more CommonMark aware. Hopefully, I can release the first commit of 2.0 in a couple of months. For the current version, I'd like to focus the limited time I can dedicate to it on priority issues. I appreciate the work that you've put here and I hope that you won't be disappointed with my response.

PhrozenByte added 5 commits September 5, 2016 04:51

Fix test/CommonMarkTest.php

e1bcc1c

Fix test/CommonMarkTest.php example regex

3a46a31

Add test/CommonMarkTestWeak.php

d33e736

Improve test/CommonMarkTestWeak.php

2cacfb8

Improve test/CommonMarkTestWeak.php

228d5f4

PhrozenByte mentioned this pull request Sep 5, 2016

Refactor PHPUnit bootstrap erusev/parsedown-extra#96

Merged

Refactor PHPUnit bootstrap

33a23fb

This allows Parsedown extensions (like Parsedown Extra) to reuse existing Parsedown tests. See erusev/parsedown-extra#96 for details.

Remove PHPUnit bootstrap in favour of composer

73dbe2f

PhrozenByte added 2 commits October 9, 2016 14:17

Add test/CommonMarkTestWeak.php to .travis.yml

f0587d4

Failing tests don't break builds on purpose, Parsedown doesn't fully comply with the CommonMark specs at the moment. We should switch to test/CommonMarkTest.php later, see erusev#423 for details.

Don't let Travis skip Parsedown's phpunit tests

be671e7

This was referenced Oct 10, 2016

Incorrect behaviour for mixed list types? #437

Closed

Improve CommonMark mixed-marker list compliance #439

Merged

Move test/CommonMarkTest.php to test/CommonMarkTestStrict.php

2423644

Add parameter `$id` to CommonMark tests

PhrozenByte mentioned this pull request Oct 12, 2016

Test Parsedown against cached expect-to-pass CommonMark spec examples #440

Closed

PhrozenByte added 2 commits October 13, 2016 22:16

Improve CommonMark spec example regex

a9f696f

CommonMark spec example [erusev#170](http://spec.commonmark.org/0.26/#example-170) has a empty HTML result.

Travis: Add PHP nightly

ae0211a

PhrozenByte mentioned this pull request Jan 23, 2017

Fix #466 #467

Open

Daniel-KM added a commit to Daniel-KM/parsedown-extra that referenced this pull request Jun 19, 2017

Reverted the upgrade for phpunit, because requires erusev/parsedown#423.

f764053

Merge branch 'master' into bugfix/CommonMarkTest

03e1a6a

Conflicts: .travis.yml test/CommonMarkTest.php test/ParsedownTest.php test/bootstrap.php

This was referenced Feb 9, 2018

Bugfix/common mark test aidanwoods/parsedown#4

Merged

Housekeeping #553

Closed

aidantwoods merged commit 48a053f into erusev:master Feb 28, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix CommonMark test #423

Fix CommonMark test #423

PhrozenByte commented Sep 5, 2016 •

edited

Loading

erusev commented Sep 5, 2016 •

edited

Loading

PhrozenByte commented Sep 5, 2016

erusev commented Sep 5, 2016

PhrozenByte commented Sep 5, 2016 •

edited

Loading

erusev commented Sep 5, 2016

PhrozenByte commented Oct 9, 2016

erusev commented Oct 9, 2016

PhrozenByte commented Oct 9, 2016

erusev commented Oct 9, 2016 •

edited

Loading

PhrozenByte commented Nov 14, 2017 •

edited

Loading

erusev commented Nov 14, 2017 •

edited

Loading

Fix CommonMark test #423

Fix CommonMark test #423

Conversation

PhrozenByte commented Sep 5, 2016 • edited Loading

erusev commented Sep 5, 2016 • edited Loading

PhrozenByte commented Sep 5, 2016

erusev commented Sep 5, 2016

PhrozenByte commented Sep 5, 2016 • edited Loading

erusev commented Sep 5, 2016

PhrozenByte commented Oct 9, 2016

erusev commented Oct 9, 2016

PhrozenByte commented Oct 9, 2016

erusev commented Oct 9, 2016 • edited Loading

PhrozenByte commented Nov 14, 2017 • edited Loading

erusev commented Nov 14, 2017 • edited Loading

PhrozenByte commented Sep 5, 2016 •

edited

Loading

erusev commented Sep 5, 2016 •

edited

Loading

PhrozenByte commented Sep 5, 2016 •

edited

Loading

erusev commented Oct 9, 2016 •

edited

Loading

PhrozenByte commented Nov 14, 2017 •

edited

Loading

erusev commented Nov 14, 2017 •

edited

Loading