Skip to content
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

A bunch of stuff for Travis CI #40

Merged
merged 5 commits into from
Mar 2, 2015
Merged

A bunch of stuff for Travis CI #40

merged 5 commits into from
Mar 2, 2015

Conversation

duncan3dc
Copy link
Member

  • Use composer update to test against the highest dependencies
  • Also test against the lowest supported dependencies
  • Added testing on the upcoming php release (7.0)
  • Use container-based travis builds

Doing this uncovered an issue in the dependencies. This looks like a problem in intervention/image, where they are misstating their lowest supported dependencies. Bumping glide's supported version to ~2.1 fixes that but I'm not sure whether you'd rather pursue a fix with intervention instead.

There's an issue with 2.0 and the versions of the laravel packages,
intervention states that it supports a lower version than it does,
which can lead to failures if users are on those versions.
@duncan3dc
Copy link
Member Author

It looks like there's another lowest dependency issue (only showing on hhvm), but I'm not sure which package is the culprit,

@reinink
Copy link
Contributor

reinink commented Feb 27, 2015

Wow @duncan3dc, this is awesome! Thanks so much for putting this together. We just need to figure out that HHVM issue. I did a quick Google search, and found this. Makes me wonder if this is related to GD. Although that doesn't make a ton of sense, since that test is mocked out pretty heavily. I'll have to do some digging.

@duncan3dc
Copy link
Member Author

Cool, I'm happy to help. I have an hhvm instance at home so I'll do some checking myself next time I get chance, if you don't beat me to the solution :)

@duncan3dc
Copy link
Member Author

It looks like the problem is @runInSeparateProcess, removing this allows the tests to pass.

Do you know why this particular test is run under this condition? The commit (dac2c96) doesn't mention it.

This is because @runInSeparateProcess has a bug in hhvm for versions prior to this
@duncan3dc
Copy link
Member Author

Adding a requirement for phpunit/php-token-stream of at least version 1.3.0 seems to resolve this issue.

There are a few issues reporting this:
sebastianbergmann/phpunit#1090
sebastianbergmann/phpunit#1214
sebastianbergmann/phpunit#1407
And I've logged a new one to see if they'll bump the php-token-stream requirement: sebastianbergmann/phpunit#1631

And the resolution seems to just be "use a more up to date version". Are you happy to list the php-token-stream as a requirement for the @runInSeparateProcess feature?

@reinink
Copy link
Contributor

reinink commented Mar 2, 2015

If I recall correctly, the @runInSeparateProcess option allows for output testing, so I suspect we'll need that still. Your solution here looks good to me!

reinink added a commit that referenced this pull request Mar 2, 2015
@reinink reinink merged commit 014474f into thephpleague:master Mar 2, 2015
@reinink
Copy link
Contributor

reinink commented Mar 2, 2015

Thanks again for your work on this @duncan3dc, much appreciated!

@duncan3dc
Copy link
Member Author

Thanks for your work on producing this cool library 👍

"league/flysystem": "~1.0",
"symfony/http-foundation": "~2.3",
"symfony/http-kernel": "~2.3"
},
"require-dev": {
"mockery/mockery": "~0.9",
"phpunit/php-token-stream": ">=1.3.0",
Copy link
Member

Choose a reason for hiding this comment

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

Don't use unbound version constraints.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not really a good point, as the dependency is only relevant for phpunit, which has a bounded version constraint, so we don't care what version of php-token-stream we get, as long as it's above 1.3.0, and compatible with our ~4.4 of phpunit, which composer takes care of

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants