Update .travis.yml to test under PHP 7+ #27
Conversation
sanmai
force-pushed
the
patch-2
branch
2 times, most recently
from
ee0a7e8 to
39742b6
Compare
|
@schmittjoh would you have a minute to review this PR? Just a line of code changed, should be a piece of cake. Thank you. |
| @@ -178,7 +178,7 @@ public function testAdd() | |||
| return 1; | |||
| }); | |||
|
|
|||
| $this->assertSame(array(0, 0, 1, $this->a, $this->b), $this->seq->all()); | |||
| $this->assertEquals(array(0, 0, 1, $this->a, $this->b), $this->seq->all()); | |||
There was a problem hiding this comment.
Could you tell me a bit more about this change? Looking at the failing test-case, it seems like the order of a and b changed which should not happen for a sequence type?
There was a problem hiding this comment.
Yeah, these hex IDs confused me, but sure they changed places. Let me see if I can find a reason.
There was a problem hiding this comment.
So what I think is happening is that sortWith uses usort which does not guarantee that elements come in specific order. And there's this commit which specifically mentions that usort may be affected, which commit went into PHP 7.0 release.
And indeed if I swap $a and $b in the input data, this specific assertion checks out all right.
One way to tackle this is to rewrite that callback used for sortWith, which is, well, somewhat difficult to follow in the current form. What do you think?
|
I prefer to keep the lock file. This is in particular useful for small libraries that do not see commits so that we have a known good state and reduce the risk of suddenly failing tests. |
|
OK, I’ll get it back in a moment. |
|
See, the problem with the commited lock file is that all packages in it have the very recent version you were able to install under currently used PHP version, which is 7.2 in my case. Hence you can see most builds on earlier versions are failing with "Your requirements could not be resolved to an installable set of packages". Now we have two options:
What do you prefer? (I went for the first option for now.) |
|
The problem was in And indeed if I swap All tests are green. |
Add more recent versions of PHP to the build at Travis. Require a specific version of PHPUnit that's in use. - Update .travis.yml to use a direct path to our vendored phpunit executable
Had this warning: $ composer validate ./composer.json is valid, but with a few warnings See https://getcomposer.org/doc/04-schema.md for details on the schema License "Apache2" is not a valid SPDX license identifier, see https://spdx.org/licenses/ if you use an open license.
- Update SequenceTest to use strings, because they're not ints, just like objects - A'n'B's are strings, let's sort them explicitly so that PHP's version differences won't come in our way
|
@schmittjoh is there anything else you'd like to change here? |
|
@schmittjoh any chance this could be merged? |
Fixes #25