lanthaler
Lets reach PHP 8 support
Summary
@lanthaler JsonLD library causes deprecations / warnings in my other projects therefore it is time to finalize PHP 8 support. The current state of this repository is some kind of a mess to be honest :sweat_smile: For instance, the test suite from https://json-ld.org/test-suite/ moved so all related tests fail. Also PHPUnit decided its a cool idea to change internals regarding DataProviders without sufficient documentation how to migrate, so I get the following stupid error messages:
ArgumentCountError: Too few arguments to function ML\JsonLD\Test\W3CTestSuiteTest::testExpansion(), 0 passed in /var/www/html/vendor/phpunit/phpunit/src/Framework/TestCase.php on line 1617 and exactly 3 expected
I tried my best to fix all issues along the way, but a closer look into my changes might be a good idea.
This PR contains already:
- PHPUnit tests run on PHP 8 using PHPUnit 10+
- many fixes for failing tests
- fixes for all PHP and PHPUnit deprecations and warnings alike
What is left to do
W3CTestSuiteTest
W3CTestSuiteTest is still failing: https://github.com/sweetrdf/json-ld/actions/runs/17466285152/job/49602746939#step:5:21 . The combination with the W3C test suite website (https://w3c.github.io/json-ld-api/tests/) makes it hard to fix because of bad documentation. I suggest to download/copy these files and add it to this repository for easier maintenance and to be independent from further sources. This should be the appropriate repository which hosts the test files from the website: https://github.com/w3c/json-ld-api
Amount of tests run differs
Also I don't know why there is a difference in the amount of tests executed on
versus
What do you think? Do you have time to help me finish this soon?
CC @zozlak At least https://github.com/sweetrdf/quickRdfIo uses this library. You might be interested in these changes. In case there is no feedback soon from the author, we could use the fork on https://github.com/sweetrdf/json-ld which is already prepared.
Thanks a lot @k00ni for working on this. We'll have to fix the W3CTestSuiteTest to ensure things keep working as expected as you said.
Back in the day I mirrored these to https://packagist.org/packages/json-ld/tests
Unfortunately that repo has been abandoned since. WDYT of creating a W3CTestSuiteLoader that sets the fields in RemoteDocument based on the .htaccess file in the original test suite or the manifest for the tests in remote-doc-manifest.jsonld?
... alternatively we could write a simple script to serve the right files/headers and bring up a local server using something like symfony.com/process. Something like
public static function setUpBeforeClass()
{
self::$process = new Process("php -S localhost:8080 -t testserver.php");
self::$process->start();
... wait till server is up ...
}
Ideally we would make those change in a separate PR against the master branch first so that we can verify that this is all working properly before we focus on updating things to PHP 8.
@lanthaler Nice to hear from you.
Unfortunately that repo has been abandoned since. WDYT of creating a W3CTestSuiteLoader that sets the fields in RemoteDocument based on the .htaccess file in the original test suite or the manifest for the tests in remote-doc-manifest.jsonld?
I have no prior knowledge and would have to take some time (which I don't really have right now) to get a grip. Based on what I saw (the abandoned test repository and the moved test suite online), they are basically big registers which contain information about inputs, expected outputs and some meta data to run a test.
If that is correct I would go with the following option:
or the manifest for the tests in remote-doc-manifest.jsonld?
That includes collecting all the test-related files in this repository and port/recreate the file-loader to run the tests. This allows us to strengthen the repository, be independent from outside sources (such as https://w3c.github.io/json-ld-api/tests/) and provide a complete base for future changes.
What is your preference here? Are you planning to invest more time to port the code to PHP 8 (with all related tasks)? How about maintenance long term? I personally would aim for a very straight forward and basic approach, which allows maintenance with limited time. As I mentioned, porting code to newer PHP versions can be very time consuming, so are you on board with focusing on PHP 8.0+ support and strip support for older PHP versions (e.g. don't test them anymore)?
But this might not the way you see for your repository. WDYT?
they are basically big registers which contain information about inputs, expected outputs and some meta data to run a test
that's correct
That includes collecting all the test-related files in this repository and port/recreate the file-loader to run the tests
the files are already installed for a dev build, see https://github.com/lanthaler/JsonLD/blob/master/composer.json#L22
This allows us to strengthen the repository, be independent from outside sources (such as https://w3c.github.io/json-ld-api/tests/) and provide a complete base for future changes.
Exactly. That's what I thought
What is your preference here? Are you planning to invest more time to port the code to PHP 8 (with all related tasks)? How about maintenance long term? I personally would aim for a very straight forward and basic approach, which allows maintenance with limited time. As I mentioned, porting code to newer PHP versions can be very time consuming, so are you on board with focusing on PHP 8.0+ support and strip support for older PHP versions (e.g. don't test them anymore)?
I also have very limited time for this unfortunately so minimizing the maintenance cost would be great. That being said, once the test suite is functional again, I hope it should should be relatively straightforward to support various PHP versions.
If that turns out not to be the case, let's discuss it based on the concrete issues we run into.
How does that sound?
Status update
I fixed a few deprecations and warnings and ported the test suite as discussed but I didn't come far :fire: In the following a short summary.
If that turns out not to be the case, let's discuss it based on the concrete issues we run into.
As far as I'm concerned, I would invest further time to get PHP 8.5 support. Older versions are out of service in the next months, so there is not enough return of invest for the time required to port the code. PHPUnit changed between v9-12 multiple times, so tests itself also might require further attention.
File transfer :heavy_check_mark:
Files were transferred from https://github.com/json-ld/tests to the local subfolder in Tests/json-ld-test-suite
Deprecations :heavy_check_mark:
Fixed a few deprecations and warnings:
- 5bb71632face798ca80a9650d84860ecf5e4e93e
- 91ca99d6835b35ea178ee244153bbb1cb3825ef0
~~Library should be almost deprecation/warning free now.~~
Porting W3CTestSuiteTest + TestManifestIterator :fire:
Started porting W3CTestSuiteTest: replaced usage of an URL and adapted the provider calls (e.g. expansionProvider)
Test suite almost fails entirely. E.g. when running
vendor/bin/phpunit --filter testExpansion
I get the following errors (tried PHPUnit 10.x and 11.x as well as with PHP 8.2 and 8.5 RC1):
There are also a lot of warnings:
whereas the line 1733 is
I have no idea how to fix these errors. My assumption is that TestManifestIterator or W3CTestSuiteTest need further changes so they can use the local files.
@lanthaler I have hit rock bottom, now its your turn again :sweat_smile:
Update :heavy_check_mark:
Getting stuck with all these failing tests and having no point to start bugged me so I tried another approach. All commits until now were replaced using a force-push. My conclusion is at the end of this comment.
Summary
-
Restarted using lowest PHP + PHPUnit version possible: PHP: 7.4 in Docker
- Reason: JSON-LD was using PHPUnit 4, but official PHP 5.6 Docker container failed to build due to "broken" URLs to Debian servers, therefore using the next lowest working versions: PHPUnit 5 with PHP 7.4
- http://json-ld.org/test-suite/tests/ was used to provide a test suite but it was moved, so I created a temporary place using my own website: http://jsonldtest.inspirito.de/test-suite/tests/ ; this allowed me at the beginning to keep most of the code as it is and to exclude this as a potential source of errors
- Either the library was buggy all along or it wasn't updated to cover the latest tests in the W3C test suite, which makes it harder to define a point in time where everything worked as expected
- for instance: testRemoteDocumentLoading > remote-doc-manifest.jsonld > t0011 failed with an empty array as result > It was because the context was not loaded properly from
option > httpLink > remote-doc-0011-context.jsonld
- for instance: testRemoteDocumentLoading > remote-doc-manifest.jsonld > t0011 failed with an empty array as result > It was because the context was not loaded properly from
- The W3C Test Suite references files which don't exist (anymore). This is strange because the W3C team usually pays attention to details.
- for instance: https://jsonldtest.inspirito.de/test-suite/tests/remote-doc-manifest.jsonld > #t0005, #t0006 and #t0007 > their input files are missing (e.g. remote-doc/0007-in.jsonld)
- Even in the official test suite this file is not available (404 error), for instance
t0007: https://w3c.github.io/json-ld-api/tests/remote-doc/0007-in.jsonld - I checked failing tests and marked them skipped with an informative error message
- Final result: PHPUnit 10.x/11.x + PHP 8.1-8.5 :rocket:
Development stages
In the following a summary about each stage I used to bump PHP/PHPUnit versions.
PHPUnit 5.x + PHP 7.4 :heavy_check_mark:
I got it to work with a few changes: 5d258d4
PHPUnit 9.6 + PHP 7.4 :fire: ===> :heavy_check_mark:
It is super weird that when upgrading PHPUnit from 5 to 9.6 all of the sudden many expected error messages change, even though the same tests ran. For this reason I moved all files from json-ld/test repository to a dedicated folder (see related commit: https://github.com/lanthaler/JsonLD/pull/113/commits/9b408f5fa0af833d00df7628b2bc99d28d21c314) and adapted the error-manifest.jsonld file using error messages coming from this library.
In the following a part of the 42 failures I received:
PHPUnit 9.6.29 by Sebastian Bergmann and contributors.
Warning: No code coverage driver available
Warning - The configuration file did not pass validation!
The following problems have been detected:
Line 11:
- Element 'phpunit', attribute 'strict': The attribute 'strict' is not allowed.
Line 19:
- Element 'filter': This element is not expected.
Test results may not be as expected.
.......................S....................................... 63 / 471 ( 13%)
............................................................... 126 / 471 ( 26%)
............................................................... 189 / 471 ( 40%)
.......................................................SSS.S..S 252 / 471 ( 53%)
FFFFFFF.FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF....S...SS.SS....... 315 / 471 ( 66%)
..SS..SSSS..................................................... 378 / 471 ( 80%)
............................................................... 441 / 471 ( 93%)
.............................. 471 / 471 (100%)
Time: 00:01.847, Memory: 12.00 MB
There were 42 failures:
1) ML\JsonLD\Test\W3CTestSuiteTest::testError with data set "https://jsonldtest.inspirito.de/test-suite/tests/error-manifest.jsonld#t0001" ('Keywords cannot be aliased to...ywords', stdClass Object (...), stdClass Object (...))
Failed asserting that exception message ' (near "@type")' contains 'keyword redefinition'.
phpvfscomposer:///var/www/html/vendor/phpunit/phpunit/phpunit:106
2) ML\JsonLD\Test\W3CTestSuiteTest::testError with data set "https://jsonldtest.inspirito.de/test-suite/tests/error-manifest.jsonld#t0002" ('A context may not include its...irect)', stdClass Object (...), stdClass Object (...))
Failed asserting that exception message 'Loading http://json-ld.org/test-suite/tests/error-0002-in.jsonld failed' contains 'loading remote context failed'.
phpvfscomposer:///var/www/html/vendor/phpunit/phpunit/phpunit:106
3) ML\JsonLD\Test\W3CTestSuiteTest::testError with data set "https://jsonldtest.inspirito.de/test-suite/tests/error-manifest.jsonld#t0003" ('A context may not include its...irect)', stdClass Object (...), stdClass Object (...))
Failed asserting that exception message 'Loading http://json-ld.org/test-suite/tests/error-0003-ctx.jsonld failed' contains 'loading remote context failed'.
phpvfscomposer:///var/www/html/vendor/phpunit/phpunit/phpunit:106
4) ML\JsonLD\Test\W3CTestSuiteTest::testError with data set "https://jsonldtest.inspirito.de/test-suite/tests/error-manifest.jsonld#t0004" ('Error dereferencing a remote context', stdClass Object (...), stdClass Object (...))
Failed asserting that exception message 'Loading tag:non-dereferencable-iri failed' contains 'loading remote context failed'.
phpvfscomposer:///var/www/html/vendor/phpunit/phpunit/phpunit:106
5) ML\JsonLD\Test\W3CTestSuiteTest::testError with data set "https://jsonldtest.inspirito.de/test-suite/tests/error-manifest.jsonld#t0005" ('Invalid remote context', stdClass Object (...), stdClass Object (...))
Failed asserting that exception message 'Loading http://json-ld.org/test-suite/tests/error-0005-in.jsonld failed' contains 'loading remote context failed'.
phpvfscomposer:///var/www/html/vendor/phpunit/phpunit/phpunit:106
[...]
FAILURES!
Tests: 471, Assertions: 720, Failures: 42, Skipped: 17.
After adapting the error manifest file most of the tests pass, some are skipped as mentioned. Here is the related commit which contains the changes: https://github.com/lanthaler/JsonLD/pull/113/commits/c1960984a99b100dcb1d278907f83e19b6096eb0
PHPUnit 10.x/11.x + PHP 8.5 :fire: ===> :heavy_check_mark:
Some adaptions were required:
- PHPUnit related changes, otherwise tests wouldn't run at all
- many PHP deprecations fixed
Related commit: https://github.com/lanthaler/JsonLD/pull/113/commits/e2238b91464f732614d4d1efed0aa035d9476f1a
Final result:
PHPUnit 12.x + PHP 8.5 :fire: :fire: :fire:
Because of internal changes in PHPUnit 11 => 12 the test providers have to be ported. Without proper changes the W3CTestSuiteTest wouldn't run at all. It might be reasonable to stop at PHPUnit 11.x for now and don't use PHPUnit 12 yet. After this PR was merged and everything settled down, it might worth another shoot.
Test related
In the following a few test (groups) which might need further attention.
Remote-Doc > t0009
W3C Test > remote-doc-manifest.jsonld > t0009 fails when using my fix for remote-doc-manifest.jsonld#t0011.
So either my fix is the problem (because without it t0009 works) or t0009 is faulty.
I didn't check the JSON-LD specification but when a propert context file is provided, why is term property
later on ignored and not part of the result?
Here are all relevant JSON files/entries:
{
"@id": "#t0009",
"@type": ["jld:PositiveEvaluationTest", "jld:ExpandTest"],
"name": "load JSON-LD document with link",
"purpose": "If a context is specified in a link header, it is not used for JSON-LD.",
"option": {
"httpLink": "<remote-doc-0009-context.jsonld>; rel=\"http://www.w3.org/ns/json-ld#context\""
},
"input": "remote-doc-0009-in.jsonld",
"expect": "remote-doc-0009-out.jsonld"
}
Context file remote-doc-0009-context.jsonld:
{
"@context": {
"@vocab": "http://example/vocab#"
}
}
Input file remote-doc-0009-in.jsonld:
[{
"@id": "",
"http://example/0009/term": "value1",
"term": "value2"
}]
Input file remote-doc-0009-out.jsonld:
[{
"@id": "https://json-ld.org/test-suite/tests/remote-doc-0009-in.jsonld",
"http://example/0009/term": [{"@value": "value1"}]
}]
Conclusion and future work
Newer PHPUnit versions require certain adaptations to the code which are not compatible with older versions. After going through all this I would say that supporting older PHP and PHPUnit versions might be possible but will be VERY time consuming.
The development stages I described earlier are more or less reflected in the commits of this PR. You can go from PHPUnit 5 up to 11 in a few steps and check the code and how it behaves for yourself.
The current state of this PR allows us now to run JSON-LD on PHP 8.1-8.5 using PHPUnit 10 or 11. This is good enough for now in my opinion. Higher PHPUnit versions or even lower ones should be considered AFTER this PR was merged. As far as I am concerned I am only interested in PHP 8.5+ for the most part.
Open TODOs
At least the following tasks remain:
- [ ] Make sure my adaptions in the tests were sufficient and didn't introduce any issues
- [ ] Replace my temporary website approach with a more permanent solution, maybe using your suggestion from before: https://github.com/lanthaler/JsonLD/pull/113#issuecomment-3272365017
- [ ] Finalize the PHP versions we want to support in the end
There is maybe more, so feel free to add it to the list.
@lanthaler Friendly ping. In case you don't have much time, please let me know what is the bare minimum to get this merged?