Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Fix PHP 7.2 tests on master branch#24515

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
nicolas-grekas wants to merge1 commit intosymfony:masterfromnicolas-grekas:php72

Conversation

@nicolas-grekas
Copy link
Member

QA
Branch?master
Bug fix?no
New feature?no
BC breaks?no
Deprecations?no
Tests pass?not yet
Fixed tickets-
LicenseMIT
Doc PR-

-php:7.1.3
-php:7.1
env:deps=high
-php:7.1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

this is meant to be reverted, right ?

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

yes, just for testing now

@stof
Copy link
Member

Looks weird to have@Simperfit and@sroze approving the PR at a time it is totally not mergeable...

sroze reacted with confused emoji

@nicolas-grekas
Copy link
MemberAuthor

nicolas-grekas commentedOct 11, 2017
edited
Loading

On 7.2:

  • HttpFoundations's NativeSessionStorageTest fails with "ini_set(): A session is active. You cannot change the session module's ini settings at this time"
  • Process hangs completely.

If someone wants to investigate, help welcomed :)
Seehttps://travis-ci.org/symfony/symfony/builds/286466244

@Simperfit
Copy link
Contributor

Simperfit commentedOct 11, 2017
edited
Loading

@nicolas-grekas Did you merge the one we did back to master ? it's the same thing

@nicolas-grekas
Copy link
MemberAuthor

Yes I did, so this is some leftover.

@Simperfit
Copy link
Contributor

Both the ini's one should not be hard to fix (I think it's the same error as the other PR).

About the Process components that hang, I could help you debugging if you want to.

@stof I approuved this PR because I though (maybe, too fast) that this will be working directly since we already worked on it. Sorry.

@stof
Copy link
Member

@Simperfit removing testing on PHP 7.1 cannot be approved as is anyway.

@sroze
Copy link
Contributor

@nicolas-grekas Interestingly I see it the other way around:

  1. The Process tests failing (fix proposed in[Process] Fix broken tests for PHP 7.2 #24516)
  2. HttpFoundation hanging. Will dig into this 🤔

@nicolas-grekas
Copy link
MemberAuthor

If it hangs locally with HttpFoundation, it may be related to an obsolete phpunit bridge. Try removing the.phpunit folder locally.

@sroze
Copy link
Contributor

sroze commentedOct 11, 2017
edited
Loading

Doesn't seam like it, I'vedev-master with your commitc754fc1 in it (tried therm option as well)

@sroze
Copy link
Contributor

Looks like all the tests tagged@runInSeparateProcess are blocking actually.

@nicolas-grekas
Copy link
MemberAuthor

composer update

@nicolas-grekasnicolas-grekas deleted the php72 branchOctober 12, 2017 09:23
nicolas-grekas added a commit that referenced this pull requestNov 6, 2017
This PR was squashed before being merged into the 4.0-dev branch (closes#24516).Discussion----------[Process] Fix broken tests for PHP 7.2| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | no| Fixed tickets |#24524,#24515| License       | MIT| Doc PR        | øFollowing#24515, trying to fix Process tests with PHP 7.2Commits-------b410a36 [Process] Fix broken tests for PHP 7.2
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@stofstofstof left review comments

+2 more reviewers

@srozesrozesroze approved these changes

@SimperfitSimperfitSimperfit approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

4.0

Development

Successfully merging this pull request may close these issues.

6 participants

@nicolas-grekas@stof@Simperfit@sroze@chalasr@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp