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

Fix PHP 7.2 tests on master branch #24515

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
6 participants
@nicolas-grekas
Member

nicolas-grekas commented Oct 11, 2017

Q A
Branch? master
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? not yet
Fixed tickets -
License MIT
Doc PR -

@nicolas-grekas nicolas-grekas added this to the 4.0 milestone Oct 11, 2017

@sroze

sroze approved these changes Oct 11, 2017

- php: 7.1.3
- php: 7.1
env: deps=high
- php: 7.1

This comment has been minimized.

@stof

stof Oct 11, 2017

Member

this is meant to be reverted, right ?

@stof

stof Oct 11, 2017

Member

this is meant to be reverted, right ?

This comment has been minimized.

@nicolas-grekas

nicolas-grekas Oct 11, 2017

Member

yes, just for testing now

@nicolas-grekas

nicolas-grekas Oct 11, 2017

Member

yes, just for testing now

@stof

This comment has been minimized.

Show comment
Hide comment
@stof

stof Oct 11, 2017

Member

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

Member

stof commented Oct 11, 2017

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

@nicolas-grekas

This comment has been minimized.

Show comment
Hide comment
@nicolas-grekas

nicolas-grekas Oct 11, 2017

Member

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 :)
See https://travis-ci.org/symfony/symfony/builds/286466244

Member

nicolas-grekas commented Oct 11, 2017

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 :)
See https://travis-ci.org/symfony/symfony/builds/286466244

@Simperfit

This comment has been minimized.

Show comment
Hide comment
@Simperfit

Simperfit Oct 11, 2017

Contributor

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

Contributor

Simperfit commented Oct 11, 2017

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

@nicolas-grekas

This comment has been minimized.

Show comment
Hide comment
@nicolas-grekas

nicolas-grekas Oct 11, 2017

Member

Yes I did, so this is some leftover.

Member

nicolas-grekas commented Oct 11, 2017

Yes I did, so this is some leftover.

@Simperfit

This comment has been minimized.

Show comment
Hide comment
@Simperfit

Simperfit Oct 11, 2017

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.

Contributor

Simperfit commented Oct 11, 2017

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

This comment has been minimized.

Show comment
Hide comment
@stof

stof Oct 11, 2017

Member

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

Member

stof commented Oct 11, 2017

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

@sroze

This comment has been minimized.

Show comment
Hide comment
@sroze

sroze Oct 11, 2017

Member

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

  1. The Process tests failing (fix proposed in #24516)
  2. HttpFoundation hanging. Will dig into this 🤔
Member

sroze commented Oct 11, 2017

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

  1. The Process tests failing (fix proposed in #24516)
  2. HttpFoundation hanging. Will dig into this 🤔
@nicolas-grekas

This comment has been minimized.

Show comment
Hide comment
@nicolas-grekas

nicolas-grekas Oct 11, 2017

Member

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

Member

nicolas-grekas commented Oct 11, 2017

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

@sroze

This comment has been minimized.

Show comment
Hide comment
@sroze

sroze Oct 11, 2017

Member

Doesn't seam like it, I've dev-master with your commit c754fc1 in it (tried the rm option as well)

Member

sroze commented Oct 11, 2017

Doesn't seam like it, I've dev-master with your commit c754fc1 in it (tried the rm option as well)

@sroze

This comment has been minimized.

Show comment
Hide comment
@sroze

sroze Oct 11, 2017

Member

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

Member

sroze commented Oct 11, 2017

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

@nicolas-grekas

This comment has been minimized.

Show comment
Hide comment
@nicolas-grekas

nicolas-grekas Oct 11, 2017

Member

composer update

Member

nicolas-grekas commented Oct 11, 2017

composer update

@nicolas-grekas nicolas-grekas deleted the nicolas-grekas:php72 branch Oct 12, 2017

nicolas-grekas added a commit that referenced this pull request Nov 6, 2017

minor #24516 [Process] Fix broken tests for PHP 7.2 (sroze)
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.2

Commits
-------

b410a36 [Process] Fix broken tests for PHP 7.2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment