Run mf2/tests test suite with PHPUnit during default testing #163

Zegnat · 2018-03-22T15:25:45Z

Maybe not ready for merge yet (although it wouldn’t hurt) as it will introduce breaking tests for TravisCI that will probably require fixes to microformats/tests rather than to the parser.

But putting the PR here so everyone can start testing and tinkering with it.

aaronpk · 2018-03-22T16:45:16Z

Is there anything in https://github.com/indieweb/php-mf2/blob/master/tests/test-suite/test-suite.php that is missing from this PR?

Zegnat · 2018-03-22T17:51:55Z

I don’t think so. I haven’t double checked. I didn’t remove it because I wasn’t sure if it might use the tests from the test-suite-data folder next to it in some way. My script does not run those.

Zegnat · 2018-03-22T22:22:24Z

@gRegorLove do you know what that test-suite-data folder contains exactly? Should those tests also be applied?

gRegorLove · 2018-03-22T23:14:59Z

I think that's outdated test data. It was added in #50 and updated to microformats/tests with #95.

I think the path will need to be updated in test-suite.php. I missed that the microformats/tests is installed from packagist now, so it's named mf2/tests.

Zegnat · 2018-03-23T08:16:06Z

I think that’s outdated test data. It was added in #50 and updated to microformats/tests with #95.

Alright, I have removed that entire folder in eb32bb7 then, as this PR runs the mf2/tests through the usual PHPUnit set already. Just need to rewrite the README to reflect this.

aaronpk · 2018-03-25T16:38:40Z

This is great, now what will it take to get all the tests to pass? 🙃

Zegnat · 2018-03-26T10:07:05Z

now what will it take to get all the tests to pass?

Time to go through all the tests one by one and fix them. From just a cursory glance it seems like a lot of failures are mistakes on the side of the tests.

The tests contain normalised datetime values, where no such normalisation should be happening according to the parsing specification. When comparing JSON to JSON, there is no longer any way to detect when a string is supposed to be a datetime (the prefixes are dropped from properties) so we can’t have PHPUnit account for this either. (I rather not hard-code properties like published.)
The tests seem to contain HTML values from e-*parsing where leading and trailing white space wasn’t stripped. So many tests with properties using the e-prefix will fail.

There is an example commit that fixes a test where both of these issues showed up.

There is also a general problem with HTML values in the tests: they are very hard to debug by hand. These test cases are supposed to test parsers, so preferably should not rely on the output of a parser either. But when there is loads of white space in the original HTML, it becomes very hard to spot the mistakes.

Zegnat · 2020-04-23T19:58:21Z

I have now added some cheat codes, because maybe we want to get this work merged before actually being able to pass all the tests. It makes no sense if all of the failing tests need to be covered by this one PR as it will grow to crazy proportions.

So what has been done now: there now exist tests for PHPUnit to run. But by default, all of them are excluded. (I am using groups for this, I could not find another way.)

Now if you as developer want to track your progress you can manually run a set of the tests, for instance by running:

./vendor/bin/phpunit --group microformats/tests/mf2

This currently runs 77 tests and fails 14 of them. You can swap out the final /mf1 for /mf1 or /mixed to run a different set of tests.

Hopefully making all of these instantly available to anyone who wants to pick up working on the PHP mf2 parser can help us go forward on working through all the different failing cases!

Opinions on merging this?

Zegnat · 2020-05-02T10:49:26Z

Apologies for the extra pushes. I rebased this branch on the current master and updated the composer.json file to point at the latest tests. Considering the troubles in #220 I fully expect to have broken composer.lock somewhere along the line and may need another commit for that 😅

In its current state, running the mf2 tests, results in the following for me:

./vendor/bin/phpunit --group microformats/tests/mf2
# … snip …
# Tests: 78, Assertions: 78, Failures: 9.

Of the 9 failures, 8 are concerning whitespace. This is after introducing new logic to ignore whitespace in 5cc7160. (Any ideas for better solving this are welcome!) Only the 9th failure seems to be the PHP parser disagreeing with the tests:

9) Mf2\Parser\Test\MicroformatsTestSuiteTest::testMf2FromTestSuite with data set "h-event/time" ('<span class="h-event">\n    <s.../span>', '{\n    "items": [{\n        "ty...: {}\n}')
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
     'items' => Array (
         0 => Array (
             'properties' => Array (
                 'end' => Array (
-                    0 => '2013-034'
-                    1 => '2013-06-27 15:34'
+                    0 => '2013-034-0800'
+                    1 => '2013-06-27 15:34-0800'
@@ @@
                     3 => '2009-06-26 19:00:00Z'
-                    4 => '2009-06-26 19:00:00'
+                    4 => '2009-06-26 19:00:00-0800'
                     5 => '2009-06-26 19:00-0800'
                     6 => '2009-06-26 19:00+0800'
                     7 => '2009-06-26 19:00Z'
-                    8 => '2009-06-26 19:00'
+                    8 => '2009-06-26 19:00-0800'
                 )
             )
             'type' => Array (...)
         )
     )
     'rel-urls' => Array ()
     'rels' => Array ()
 )

Not a lot left I can think of doing with this PR … all reviews and input welcomed!

Zegnat · 2020-05-09T13:38:46Z

I have now tweaked it a bit more and basically reverted the parser to use the textContent logic of before the e8da04f merge of whitespace normalisation. This makes it a lot easier to compare things without guessing.

Also happy to say that, with this in place, there are only 2 failures when running the mf2 tests! 🎉 One of those seems to be a problem in the test suite and I have opened a PR for it: microformats/tests#117. The other one is the date time normalisation difference mentioned above.

I would still love to get this merged before passing all the tests, so we can manually run them to check progress during development. Alternatively maybe it is worth holding the merge until we can remove the microformats/tests/mf2 exclusion in phpunit.xml. But I am not sure anyone is currently looking into that bug at all.

Zegnat · 2020-05-09T19:39:28Z

Travis now automatically runs the mixed tests! 🎉

The reason the h-event/time test is failing is because the PHP parser has supported a version of a proposed spec extension for a number of years now. So either we have to put this behind a version flag, or lobby for it to land in the spec, if we want to be able to pass that test at all.

I am thinking I may mark that specific test as one that should be skipped. Then we can make Travis run all the mf2 tests as well!

Zegnat · 2020-05-09T20:26:55Z

Hmm. I am going to need some help debugging this.

Travis is not passing the tests, but locally I am passing them all. Both with PHP 7.4 (which Travis is not yet testing, pending #220) and also with PHP 7.3 (which is failing in Travis). Anyone see why Travis is having problems here?

php --version
# PHP 7.4.4 (cli) (built: Mar 19 2020 20:12:27) ( NTS )
# Copyright (c) The PHP Group
# Zend Engine v3.4.0, Copyright (c) Zend Technologies
#     with Zend OPcache v7.4.4, Copyright (c), by Zend Technologies
./vendor/bin/phpunit
# PHP Deprecated:  The each() function is deprecated. This message will be suppressed on further calls in /Users/martijn/Source/php-mf2/vendor/phpunit/phpunit/src/Util/Getopt.php on line 38
# 
# Deprecated: The each() function is deprecated. This message will be suppressed on further calls in /Users/martijn/Source/php-mf2/vendor/phpunit/phpunit/src/Util/Getopt.php on line 38
# PHPUnit 4.8.35 by Sebastian Bergmann and contributors.
# 
# ...............................................................  63 / 397 ( 15%)
# ..................................................I............ 126 / 397 ( 31%)
# ............................................................... 189 / 397 ( 47%)
# ............................................................... 252 / 397 ( 63%)
# .....................................................S......... 315 / 397 ( 79%)
# ............................................................... 378 / 397 ( 95%)
# ...................
# 
# Time: 9.15 seconds, Memory: 8.00MB
# 
# OK, but incomplete, skipped, or risky tests!
# Tests: 397, Assertions: 868, Skipped: 1, Incomplete: 1.

/usr/local/Cellar/php@7.3/7.3.16/bin/php --version
# PHP 7.3.16 (cli) (built: Mar 19 2020 11:19:09) ( NTS )
# Copyright (c) 1997-2018 The PHP Group
# Zend Engine v3.3.16, Copyright (c) 1998-2018 Zend Technologies
#     with Zend OPcache v7.3.16, Copyright (c) 1999-2018, by Zend Technologies
/usr/local/Cellar/php@7.3/7.3.16/bin/php ./vendor/bin/phpunit
# PHP Deprecated:  The each() function is deprecated. This message will be suppressed on further calls in /Users/martijn/Source/php-mf2/vendor/phpunit/phpunit/src/Util/Getopt.php on line 38
# 
# Deprecated: The each() function is deprecated. This message will be suppressed on further calls in /Users/martijn/Source/php-mf2/vendor/phpunit/phpunit/src/Util/Getopt.php on line 38
# PHPUnit 4.8.35 by Sebastian Bergmann and contributors.
# 
# ...............................................................  63 / 397 ( 15%)
# ..................................................I............ 126 / 397 ( 31%)
# ............................................................... 189 / 397 ( 47%)
# ............................................................... 252 / 397 ( 63%)
# .....................................................S......... 315 / 397 ( 79%)
# ............................................................... 378 / 397 ( 95%)
# ...................
# 
# Time: 24.99 seconds, Memory: 8.00MB
# 
# OK, but incomplete, skipped, or risky tests!
# Tests: 397, Assertions: 868, Skipped: 1, Incomplete: 1.

gRegorLove · 2020-05-26T23:48:56Z

@Zegnat It looks like it's whitespace differences in the failing tests (e.g. in the PHP 5.6 env).

Maybe Travis isn't getting the correct commit for microformats/tests? #220 is now merged and runs composer update so maybe rebasing would make these pass.

As discussed previously php-mf2 has custom handling of textContent to match with what consumers were expecting to get back. This means all tests around plain text output have a high tendency to fail. This change addresses that a little by ignoring whitespace differences within e-* properties.

Travis was showing weird errors in PHP versions before 7.0 about duplicate name usage. Weird because the Parser class defined for the test should live in a completely separate namespace.

We mark a single test as incomplete (ie. unimplemented) because the parser implements a proposed extension to the mf2 specification while the test suite has tests that exactly match the specification.

Zegnat · 2020-05-27T08:28:19Z

@gRegorLove rebased. Tests still passing for me locally on my machine, tests still not passing on Travis.

It is also interesting how it seems to fail on in-HTML whitespace, that isn’t something we should be touching at all, right? Also interesting how it does pass when it uses the user-land HTML parser. Almost makes me wonder if there is something peculiar with the combination of PHP version and libxml (libxml2?) version that Travis is using versus those that I am using.

Is anyone able to recreate the Travis failures locally for further inspection?

@cweiske

Certain builds of PHP seem to drop specific whitespace during the HTML parsing step. There seems to be no reason for this and the behaviour has been seen for versions of PHP ranging all the way from 5.6 to 7.3. The behaviour seems to be sidestepped by providing any supported parsing option to the loadHTML method. LIBXML_NOWARNING was chosen as it seemed like it would have the least impact overall. For a PHP test to surface the behaviour, as well as the test of the effect of constants please see: https://gist.github.com/Zegnat/a94489e9b7d5501193e724e336bc6052 Huge thanks to everyone in #indieweb-dev who went on this journey with me! Especially @cweiske and @Lewiscowles1986 for all the extra testing, and @gRegorLove for getting the ball rolling with parsing options.

Zegnat · 2020-05-30T11:14:15Z

Everything is green! Thanks everyone who contributed to this, big team effort! 🎉

The only tests not running right now are the mf1-only tests. If you are interested in seeing these, you can run:

vendor/bin/phpunit --group microformats/tests/mf1

This will currently result in a lot of failures:

FAILURES!
Tests: 39, Assertions: 39, Failures: 27.

Hopefully landing this PR will help reviewing future fixes. Should make checking fixes like those in #201 much easier.

Zegnat · 2020-05-30T11:22:14Z

@gRegorLove you mentioned your web server actually had this problem. Do you think you could run this branch there and see if it fixed things?

If running the full test suite is problematic for whatever reason, you can also try the lookingforoptions.php from my testing gist. That will try as many constants as possible to see if any of them would fix the issue on your environment. If LIBXML_NOWARNING comes out with a true that means my latest commit would have fixed it for your environment.

gRegorLove · 2020-06-07T21:25:07Z

@Zegnat LIBXML_NOWARNING is true

No options            false
Explicit zero         false (0)
LIBXML_BIGLINES       true  (4194304)
LIBXML_COMPACT        true  (65536)
LIBXML_DTDATTR        true  (8)
LIBXML_DTDLOAD        true  (4)
LIBXML_DTDVALID       true  (16)
LIBXML_HTML_NOIMPLIED true  (8192)
LIBXML_HTML_NODEFDTD  true  (4)
LIBXML_NOBLANKS       false (256)
LIBXML_NOCDATA        true  (16384)
LIBXML_NOENT          true  (2)
LIBXML_NOERROR        true  (32)
LIBXML_NONET          true  (2048)
LIBXML_NOWARNING      true  (64)
LIBXML_NOXMLDECL      true  (2)
LIBXML_NSCLEAN        true  (8192)
LIBXML_PARSEHUGE      true  (524288)
LIBXML_PEDANTIC       true  (128)
LIBXML_XINCLUDE       true  (1024)
LIBXML_SCHEMA_CREATE  true  (1)

Built-in tests are passing:

php --version
# PHP 7.2.30 (cli) (built: Apr 24 2020 01:29:53) ( NTS )
# Copyright (c) 1997-2018 The PHP Group
# Zend Engine v3.2.0, Copyright (c) 1998-2018 Zend Technologies
#     with Zend OPcache v7.2.30, Copyright (c) 1999-2018, by Zend Technologies
./vendor/bin/phpunit
# ...snip...
# OK, but incomplete, skipped, or risky tests!
# Tests: 317, Assertions: 790, Skipped: 1.

The test suite grouping is reporting no tests executed:

./vendor/bin/phpunit --group microformats/tests/mf2
# PHPUnit 4.8.36 by Sebastian Bergmann and contributors.
# ...snip...
# No tests executed!

Same result when I run ./vendor/bin/phpunit --group microformats/tests/mf1

Zegnat · 2020-06-08T18:42:37Z

Are you sure you are on the correct branch / commit? Running just bare PHPUnit for me results in ~80 more tests and ~100 more assertions:

php --version
# PHP 7.4.4 (cli) (built: Mar 19 2020 20:12:27) ( NTS )
# Copyright (c) The PHP Group
# Zend Engine v3.4.0, Copyright (c) Zend Technologies
#     with Zend OPcache v7.4.4, Copyright (c), by Zend Technologies
./vendor/bin/phpunit
# PHP Deprecated:  The each() function is deprecated. This message will be suppressed on further calls in /Users/martijn/Source/php-mf2/vendor/phpunit/phpunit/src/Util/Getopt.php on line 38
# 
# Deprecated: The each() function is deprecated. This message will be suppressed on further calls in /Users/martijn/Source/php-mf2/vendor/phpunit/phpunit/src/Util/Getopt.php on line 38
# PHPUnit 4.8.36 by Sebastian Bergmann and contributors.
# 
# ...............................................................  63 / 397 ( 15%)
# ..................................................I............ 126 / 397 ( 31%)
# ............................................................... 189 / 397 ( 47%)
# ............................................................... 252 / 397 ( 63%)
# .....................................................S......... 315 / 397 ( 79%)
# ............................................................... 378 / 397 ( 95%)
# ...................
# 
# Time: 16.43 seconds, Memory: 8.00MB
# 
# OK, but incomplete, skipped, or risky tests!
# Tests: 397, Assertions: 868, Skipped: 1, Incomplete: 1.
/usr/local/Cellar/php@7.3/7.3.16/bin/php --version
# PHP 7.3.16 (cli) (built: Mar 19 2020 11:19:09) ( NTS )
# Copyright (c) 1997-2018 The PHP Group
# Zend Engine v3.3.16, Copyright (c) 1998-2018 Zend Technologies
#     with Zend OPcache v7.3.16, Copyright (c) 1999-2018, by Zend Technologies
/usr/local/Cellar/php@7.3/7.3.16/bin/php ./vendor/bin/phpunit
# PHP Deprecated:  The each() function is deprecated. This message will be suppressed on further calls in /Users/martijn/Source/php-mf2/vendor/phpunit/phpunit/src/Util/Getopt.php on line 38
# 
# Deprecated: The each() function is deprecated. This message will be suppressed on further calls in /Users/martijn/Source/php-mf2/vendor/phpunit/phpunit/src/Util/Getopt.php on line 38
# PHPUnit 4.8.36 by Sebastian Bergmann and contributors.
# 
# ...............................................................  63 / 397 ( 15%)
# ..................................................I............ 126 / 397 ( 31%)
# ............................................................... 189 / 397 ( 47%)
# ............................................................... 252 / 397 ( 63%)
# .....................................................S......... 315 / 397 ( 79%)
# ............................................................... 378 / 397 ( 95%)
# ...................
# 
# Time: 12.53 seconds, Memory: 8.00MB
# 
# OK, but incomplete, skipped, or risky tests!
# Tests: 397, Assertions: 868, Skipped: 1, Incomplete: 1.

I’ll see if I can check with a PHP 7.2 shortly too. But I have a hard time understanding why PHPUnit would parse the tests differently.

Travis’ PHP 7.2 run is reporting 2 more tests than even my local run:

OK, but incomplete, skipped, or risky tests!
Tests: 399, Assertions: 871, Skipped: 1, Incomplete: 1.
The command "$TRAVIS_BUILD_DIR/vendor/bin/phpunit" exited with 0.

I wonder if you are seeing a problem with the groups not running because you aren’t running any of the test suit tests 🤔

gRegorLove · 2020-06-08T19:38:39Z

Oops, I was on master. On central-tests it's working.

php --version
# PHP 7.2.30 (cli) (built: Apr 24 2020 01:29:53) ( NTS )
composer phpunit
> ./vendor/bin/phpunit
# PHPUnit 4.8.36 by Sebastian Bergmann and contributors.
# OK, but incomplete, skipped, or risky tests!
# Tests: 399, Assertions: 871, Skipped: 1, Incomplete: 1.

Grouping is running now and getting same result as you showed above.

./vendor/bin/phpunit --group microformats/tests/mf1
# FAILURES!
# Tests: 39, Assertions: 39, Failures: 27.

Zegnat · 2020-06-08T19:41:45Z

That’s what I was expecting to see! Good to hear it is now working on your PHP 7.2 too!

gRegorLove · 2020-06-08T23:00:35Z

Awesome. Is there anything else I should test with this PR or is it ready to merge now?

Zegnat · 2020-06-09T07:50:45Z

I think it is ready for merge. It already does more than I originally set out to do, now that Travis can run both the mf2 and mixed tests automatically. I don't think it helps us to keep off on merging until mf1 tests all pass. Better to iterate on that while having access to the tests merged in.

So if people are happy with the code: merge away 🚀 Thanks for sticking with this PR!

Zegnat force-pushed the central-tests branch from 4e3a7ad to 48a71c8 Compare August 24, 2018 08:32

Zegnat force-pushed the central-tests branch from 48a71c8 to 614686d Compare April 23, 2020 13:40

gRegorLove added this to the 0.5.0 milestone Apr 26, 2020

Zegnat force-pushed the central-tests branch 2 times, most recently from d69f687 to 853b3cc Compare May 2, 2020 10:40

Zegnat requested review from dshanske and gRegorLove May 2, 2020 10:50

Zegnat added 10 commits May 27, 2020 10:09

Let PHPUnit validate the parser against the mf2/tests suite

16c2595

Remove outdated test data

2d026e5

Detect when the parser is giving back too many properties

b8cc617

Have PHPUnit display better test failures

a9d589a

Better display of test name

8025feb

Assume all HTML to originate on example.com

d36841c

Hard link to latest version of the test suite

cc2ee57

Split tests per directory, exclude them unless explicitly triggered

236e2d9

Point at latest test suite

578ad39

Zegnat added 6 commits May 27, 2020 10:10

Run parser with old textContent logic to pass interoperable tests

4d46586

Rename Parser to TestSuiteParser

fb6d089

Travis was showing weird errors in PHP versions before 7.0 about duplicate name usage. Weird because the Parser class defined for the test should live in a completely separate namespace.

Fix bug in original image replacement code

79eb0a5

No longer need to exclude mixed tests: all pass!

bbda674

Update the lock file to the latest test suite version

727a299

Make Travis run all mf2 tests

784b6a6

We mark a single test as incomplete (ie. unimplemented) because the parser implements a proposed extension to the mf2 specification while the test suite has tests that exactly match the specification.

Zegnat force-pushed the central-tests branch from ce47c15 to 784b6a6 Compare May 27, 2020 08:12

aimee-gm mentioned this pull request May 29, 2020

Collapse space when implying value from textContents microformats/microformats-parser#51

Closed

gRegorLove merged commit 1e2a2ee into microformats:master Jun 13, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Run mf2/tests test suite with PHPUnit during default testing #163

Run mf2/tests test suite with PHPUnit during default testing #163

Zegnat commented Mar 22, 2018

aaronpk commented Mar 22, 2018

Zegnat commented Mar 22, 2018

Zegnat commented Mar 22, 2018

gRegorLove commented Mar 22, 2018

Zegnat commented Mar 23, 2018

aaronpk commented Mar 25, 2018

Zegnat commented Mar 26, 2018

Zegnat commented Apr 23, 2020

Zegnat commented May 2, 2020

Zegnat commented May 9, 2020

Zegnat commented May 9, 2020

Zegnat commented May 9, 2020

gRegorLove commented May 26, 2020 •

edited

Loading

Zegnat commented May 27, 2020

Zegnat commented May 30, 2020

Zegnat commented May 30, 2020

gRegorLove commented Jun 7, 2020 •

edited

Loading

Zegnat commented Jun 8, 2020

gRegorLove commented Jun 8, 2020

Zegnat commented Jun 8, 2020

gRegorLove commented Jun 8, 2020

Zegnat commented Jun 9, 2020

Run mf2/tests test suite with PHPUnit during default testing #163

Run mf2/tests test suite with PHPUnit during default testing #163

Conversation

Zegnat commented Mar 22, 2018

aaronpk commented Mar 22, 2018

Zegnat commented Mar 22, 2018

Zegnat commented Mar 22, 2018

gRegorLove commented Mar 22, 2018

Zegnat commented Mar 23, 2018

aaronpk commented Mar 25, 2018

Zegnat commented Mar 26, 2018

Zegnat commented Apr 23, 2020

Zegnat commented May 2, 2020

Zegnat commented May 9, 2020

Zegnat commented May 9, 2020

Zegnat commented May 9, 2020

gRegorLove commented May 26, 2020 • edited Loading

Zegnat commented May 27, 2020

Zegnat commented May 30, 2020

Zegnat commented May 30, 2020

gRegorLove commented Jun 7, 2020 • edited Loading

Zegnat commented Jun 8, 2020

gRegorLove commented Jun 8, 2020

Zegnat commented Jun 8, 2020

gRegorLove commented Jun 8, 2020

Zegnat commented Jun 9, 2020

gRegorLove commented May 26, 2020 •

edited

Loading

gRegorLove commented Jun 7, 2020 •

edited

Loading