Tests are not running

[panique]

The Unit Tests of this project are not running, and the documentation seems to be outdated, so I hereby want to change that to make it smooth and easy to commit fixes with proper tests to the project :)

What's the problem:

1. The official documentation show an uncommon, undocumented way to run the tests, but if done exactly like described it will not work (vendor/bin/atoum), so the documentation needs some work
2. pdfparser doesn't use PHPUnit directly, but phpunit-bridge / simple-phpunit, but it's a) undocumented how to use it and b) fails when running vendor/bin/simple-phpunit with the error PHP Fatal error: Declaration of Tests\Smalot\PdfParser\TestCase::setUp() must be compatible with PHPUnit\Framework\TestCase::setUp(). Until today I was not able to run the tests of pdfparser.

Why this might be important:

Currently it's hard to commit fixes with proper tests to the project. Making the tests run out-of-the-box will encourage people to take part in the development.

Used enviroment

PHP 7.4.12

please correct me if I'm overseeing something here ;)

[panique]

I found the problem and made a patch:

Problem

pdfparser's tests were imcompatible with PHP 7.4

Fix

This simply adds PHP 7.4 compatibility (but might make it incompatible with older 5.x versions (?)) and also fixes some PHPUnit 9 deprecation warnings.

pdfparser-fix-for-PHP-7.4.patch.txt

Feel free to use this :)

[k00ni]

@panique thank you for the feedback.

You are right that the documentation is not in sync with the current state of the library. @smalot doesn't respond to pings lately, so we other co-maintainers can't help here unfortunately.

Because the library supports PHP 5.6+ (ref) we need the PHPUnit bridge you mentioned, because it runs the appropriate PHPUnit version based on your PHP version.

There must be a misunderstanding though, because tests run in PHP 7.4, for example: https://travis-ci.org/github/smalot/pdfparser/jobs/744424658

php vendor/bin/simple-phpunit -v $COVERAGE_FLAGS
PHPUnit 7.5.20 by Sebastian Bergmann and contributors.
Runtime:       PHP 7.4.12 with Xdebug 2.9.8
Configuration: /home/travis/build/smalot/pdfparser/phpunit.xml
Testing all
...............................................................  63 / 110 ( 57%)
...............................................                 110 / 110 (100%)
Time: 16.88 seconds, Memory: 48.00 MB

My guess is that you tried to run a PHPUnit version which is too new. Please check our travis.yml, maybe we oversaw something.

Making the tests run out-of-the-box will encourage people to take part in the development.

I am using a docker container with a simple PHP setup to develop on my local machine (PHP 7.3 currently). Running composer update and then php vendor/bin/simple-phpunit is enough to setup and run all tests.

Thank you for the work on the patch, but I don't have the time to manually deploy it. Please use pull requests for that. Besides, I don't think this patch is required, because our tests run on PHP 7.4.

[panique]

Hi, thanks, you are right! The problem was PHPUnit 9.4, not PHP itself!

[k00ni]

pdfparser's tests were imcompatible with PHP 7.4

I can approve that there is a problem when running tests with PHP 7.3+ and PHP bridge. We didn't see them in our tests because we manually set SYMFONY_PHPUNIT_VERSION to 7.5. But if it is unset, it defaults to 8.3 and causes

PHP Fatal error: Declaration of Tests\Smalot\PdfParser\TestCase::setUp() must be compatible with PHPUnit\Framework\TestCase::setUp()

@j0k3r is that fixable or do we have to abandon PHP 5.6 support first?

[j0k3r]

I'm OK to ditch PHP 5.6. PHP 8 is around the corner, it's time to move forward.

[smalot]

Hi Guys
@k00ni what about removing PHP 5.6 support ?
But it will create a BC Break
May be we should create a new major release ?

[smalot]

How can I help you ?

[j0k3r]

Maybe we can just update the code to "php": ">=7.2", or maybe 7.3 regarding:

[k00ni]

I can life with either way (remove it OR keep it + adapt test tooling). It is related to #383 (comment)

[k00ni]

What is the status here? Can it be closed @panique?