Skip to content

Test PHP 7.4 on Travis CI - #40

Closed
WyriHaximus wants to merge 3 commits into
reactphp:masterfrom
WyriHaximus-labs:test-php-7.4
Closed

Test PHP 7.4 on Travis CI#40
WyriHaximus wants to merge 3 commits into
reactphp:masterfrom
WyriHaximus-labs:test-php-7.4

Conversation

@WyriHaximus

@WyriHaximusWyriHaximus commented Jan 31, 2020

Copy link
Copy Markdown
Member

Required for #37, running into one failing test with it, only on PHP 7.4 due to GC improvements.

@WyriHaximus

Copy link
Copy Markdown
MemberAuthor

Interestingly enough #37 is fine, the issue is here already: https://travis-ci.org/reactphp/promise-timer/jobs/644555479#L552

@clue

clue commented Feb 1, 2020

Copy link
Copy Markdown
Member

@WyriHaximus Interesting, the garbage references were believed to be fixed with #33/#34. I will look into the garbage references and what could cause a possible regression for PHP 7.4 only 👍

https://travis-ci.org/reactphp/promise-timer/jobs/644562579#L507

@WyriHaximus

Copy link
Copy Markdown
MemberAuthor

@clue Opened reactphp/promise#157 upstream, and there are two failing tests regarding to GC's there as well

@WyriHaximus

Copy link
Copy Markdown
MemberAuthor

@clue Also, this is the bug report I found yesterday when looking at PHP 7.4 specific GC changes: https://bugs.php.net/bug.php?id=72530

@clue

clue commented Feb 5, 2020

Copy link
Copy Markdown
Member

@WyriHaximus Thanks for reporting this upstream in reactphp/promise#157 👍 I can not currently reproduce the problem upstream or here, so I think this is best addressed upstream first:

$ docker run -it --rm -v `pwd`:/data --workdir=/data php:7.4 php vendor/bin/phpunit

@WyriHaximus

Copy link
Copy Markdown
MemberAuthor

@clue yup, trying to find out why, but things seem to be point to the enqueue function

@WyriHaximus
WyriHaximusforce-pushed the test-php-7.4 branch 9 times, most recently from d10e897 to fa87e75CompareFebruary 22, 2020 22:27
We're doing this because certain tests have memory leaks only when code coverage is collected.
@WyriHaximus

Copy link
Copy Markdown
MemberAuthor

@clue@jsor Ok this is also green, took the same approach as reactphp/promise#157

@jsorjsor closed this in #41 Mar 5, 2020
@clueclue modified the milestones: v1.5.1, v1.6.0Jul 10, 2020
Sign up for freeto 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.

2 participants

@WyriHaximus@clue