milesj / decoda

A lightweight lexical string parser for BBCode styled markup.
MIT License
196 stars 52 forks source link

Adding test on ClickableHook which fails in 6.8.0 #125

Closed lgiraudel closed 7 years ago

lgiraudel commented 7 years ago

Hello,

I noticed something between the 6.7.2 and 6.8.0 versions which changes the behavior regarding the <br/> tags.

I didn't found a fix for now, but here's a phpunit test showing the error.

In 6.7.2, everything is fine:

./vendor/bin/phpunit -c phpunit.xml.dist
PHPUnit 4.8.35 by Sebastian Bergmann and contributors.

...............................................................  63 / 215 ( 29%)
............................................................... 126 / 215 ( 58%)
............................................................... 189 / 215 ( 87%)
..........................

Time: 472 ms, Memory: 21.75MB

But in 6.8.0:

./vendor/bin/phpunit -c phpunit.xml.dist
PHPUnit 4.8.35 by Sebastian Bergmann and contributors.

...............................................................  63 / 223 ( 28%)
............................................................... 126 / 223 ( 56%)
F.............................................................. 189 / 223 ( 84%)
..................................

Time: 499 ms, Memory: 23.00MB

There was 1 failure:

1) Decoda\Hook\ClickableHookTest::testUrlParsing
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'<br/><a href="http://domain.com">http://domain.com</a>'
+'<br/>http://domain.com'

/Users/loic/workspace/decoda/tests/Decoda/Hook/ClickableHookTest.php:59

Is this change wanted?

vladrusu commented 7 years ago

Hello ! The test is bogus. From v6.8.0, all the "magic" (replacing) in ClickableHook is made in beforeParse, and not in afterParse. So you should check your test against ->beforeParse(text) and see if there's a problem. Anyways, I pretty heavily tested an upcoming update (6.8.1), for which I created a pull request just earlier, and I saw no problems whatsoever.

milesj commented 7 years ago

Awesome, closing this!