Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Tests] Polyfills not loaded #425

Closed
IonBazan opened this issue Jan 26, 2023 · 6 comments
Closed

[Tests] Polyfills not loaded #425

IonBazan opened this issue Jan 26, 2023 · 6 comments

Comments

@IonBazan
Copy link
Contributor

When running the test suite on my local environment (PHP 8.1), the polyfill is not loaded correctly, causing the new functions to be not registered. Interestingly, the test is marked as skipped, instead of error/warning 🤔

I bet it has something to do with the custom test runner/hooks but not sure what's wrong there.

vendor/bin/simple-phpunit --filter php83 -vvv
PHPUnit 9.5.26 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.1.14
Configuration: /Users/ionbazan/work/polyfill/phpunit.xml.dist
Warning:       Your XML configuration validates against a deprecated schema.
Suggestion:    Migrate your XML configuration using "--migrate-configuration"!

Testing
SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS                        42 / 42 (100%)

Time: 00:00.317, Memory: 74.00 MB

There were 42 skipped tests:

1) Symfony\Polyfill\Tests\Php83\Php83Test::testJsonValidate with data set #0 (false, '', 'Syntax error')
Internal function not found: json_validate

/Users/ionbazan/work/polyfill/tests/Php83/Php83Test.php:24

2) Symfony\Polyfill\Tests\Php83\Php83Test::testJsonValidate with data set #1 (false, '.', 'Syntax error')
Internal function not found: json_validate

/Users/ionbazan/work/polyfill/tests/Php83/Php83Test.php:24

[...]
@nicolas-grekas
Copy link
Member

That's how it's supposed to work to me: the internal function (the native one, non-polyfilled) is not found since you're on PHP 8.1
Try running this on PHP 8.3, it should be green.

@IonBazan
Copy link
Contributor Author

That doesn't make much sense to me. From what I remember, we were including bootstrap.php files using TestListenerTrait which made sure the function is registered globally - this is to use same test code on PHP 8.1 and 8.3 and compare results between polyfilled and native implementation.

@IonBazan
Copy link
Contributor Author

OK, I think I got it now - I always thought we run the future-compatible tests on old versions too, and that the functions are globally registered but now I see the mechanism is much more sophisticated. I just wonder if that won't produce any regressions, as we test json_validate only on PHP 8.3 and up, we could technically put some new syntax which is not compatible with older versions and still observe a green CI 🤔

@stof
Copy link
Member

stof commented Jan 31, 2023

Aren't we testing the polyfill variant on all PHP versions ?

@nicolas-grekas
Copy link
Member

FTR this works with PHPUnit 7.5 (and PHP 7.4)

SYMFONY_PHPUNIT_VERSION=7.5 ./vendor/bin/simple-phpunit

We'd need to figure out what changed in PHPUnit 8+

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Sep 9, 2024

Looks like using phpunit 8.5 works. Fixed by #495 then.
See #495 that adds a ./phpunit to help with running phpunit 8.5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants