Skip to content

Add Symfony back in the community test suite #4666

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

Closed
wants to merge 2 commits into from

Conversation

nicolas-grekas
Copy link
Contributor

It was disabled in #1054, but this should make it OK.
Feel free to ping me when you have any issue with this test suite, I'd be happy to help maintain it.

@nicolas-grekas
Copy link
Contributor Author

I'm not sure where I should read the output for the related jobs? Could anyone point me at past ones?

@nikic
Copy link
Member

nikic commented Sep 2, 2019

Symfony is still enabled on 7.4, the reason why I dropped it on master is that phpunit doesn't install due to platform requirements (it is not compatible with PHP 8 and there is no way to pass --ignore-platform-reqs). Unless this got fixed in the phpunit wrapper, this issue will appear again.

You can edit https://github.com/php/php-src/blob/master/azure-pipelines.yml and drop everything but the community job to check whether it works. It's not run on pull request builds.

@nicolas-grekas
Copy link
Contributor Author

nicolas-grekas commented Sep 2, 2019

it is not compatible with PHP 8 and there is no way to pass --ignore-platform-reqs

got it, the attached patch should work around the issue, don't you think?

@nikic
Copy link
Member

nikic commented Sep 2, 2019

What is the relation between flex and the memory_limit? We can also change the memory_limit.

It is possible to run php7.3 ./phpunit install? I'd guess that would avoid both the PHP version issue and be faster. I just didn't realize that the installation part could be run separately from the tests.

@nicolas-grekas
Copy link
Contributor Author

What is the relation between flex and the memory_limit?

When there are too many legacy tags, the matrix explodes, see symfony/flex#378 which introduced the workaround exactly for this reason.

@nicolas-grekas
Copy link
Contributor Author

I pushed symfony/symfony@62020ab on the bridge to give it the flexibility we need here.

Be careful if you want to do further tweaks, I push-forced on my branch.

@nikic
Copy link
Member

nikic commented Sep 2, 2019

Yay, looks like it works.

Also one ubsan warning (also on 7.4 but got lost in the log): ./home/vsts/work/1/s/ext/standard/random.c:238:13: runtime error: signed integer overflow: 9223372036854775807 - -9223372036854775808 cannot be represented in type 'long int'

Should probably configure it to abort instead of only printing a message.

@nicolas-grekas
Copy link
Contributor Author

Cool! What's the next step? There are failures, errors, deprecation notices (a bunch of them, dunno why as I don't have them on 7.3)

@KalleZ
Copy link
Member

KalleZ commented Sep 2, 2019

The next step should just be to merge it, we don’t really care about deprecations, notices etc on a userland level but the results of the ub tooling on the test suites :)

@nikic
Copy link
Member

nikic commented Sep 12, 2019

Partially backported via #4672 and then merged up. The symfony run was successful in https://dev.azure.com/phpazuredevops/PHP/_build/results?buildId=2567.

@nikic nikic closed this Sep 12, 2019
@nicolas-grekas nicolas-grekas deleted the test-symfony branch June 19, 2022 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants