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

lets just run simple-phpunit #1027

Closed
wants to merge 4 commits into from
Closed

lets just run simple-phpunit #1027

wants to merge 4 commits into from

Conversation

dbu
Copy link
Member

@dbu dbu commented Dec 20, 2017

Q A
Branch? 2.0
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? => travis
Fixed tickets -
License MIT
Doc PR -

Trying to fix setup of travis testing.

@dbu dbu added the State: Reviewing This item is being reviewed to determine if it should be accepted. label Dec 20, 2017
@sebastianblum
Copy link
Contributor

sebastianblum commented Dec 21, 2017

@dbu David, in der composer.json ist das bin-dir mit "bin" anstatt "vendor/bin" angegeben.
Du müsstest die Tests ohne ./vendor/ laufen lassen oder die composer.json anpassen.

@dbu
Copy link
Member Author

dbu commented Dec 21, 2017

in der composer.json ist das bin-dir mit "bin" anstatt "vendor/bin" angegeben.
Du müsstest die Tests ohne ./vendor/ laufen lassen oder die composer.json anpassen.

nur für branch 1.0, dieser merge request ist für 2.0 und da wurde die bin config entfernt. würde das phpunit binary nicht gefunden, müsste auch ein fehler ausgegeben werden.

@dbu
Copy link
Member Author

dbu commented Dec 22, 2017

@lsmith77 would you have an idea why this is not running any tests? locally it worked for me.

@lsmith77
Copy link
Contributor

seem to be passing now .. ?

@cedricziel
Copy link
Collaborator

Passing nur IMHO without running any tests. There's no output

@lsmith77
Copy link
Contributor

ah .. strange

@sebastianblum
Copy link
Contributor

@dbu & @lsmith77 the problem is in my opinion, that symfony/phpunit-bridge runs phpunit 5.7 at php 7.1 (there are no tests executed) and phpunit 6.5 at 7.2 (tests are fine)

I'm trying a lot in this pull request #1029 but locally it is running because of phpunit ^6.0 in the composer.json

@dbu
Copy link
Member Author

dbu commented Jan 2, 2018

if we can move everything to phpunit 6 only, thats fine too. we don't need to support different versions of phpunit or anything, the tests are just for this bundle itself. does #1029 replace this PR?

@sebastianblum
Copy link
Contributor

no @dbu #1029 has too much changes and should be deleted.
If I have a solution, you can update this request or I will create a new clean pull request.

@sebastianblum
Copy link
Contributor

@dbu can you please have a look at https://travis-ci.org/liip/LiipImagineBundle/builds/324187910
now #1029 looks much better

@dbu
Copy link
Member Author

dbu commented Jan 2, 2018

that looks indeed a lot better. i would be happy if we just merge #1029, no need to further split that up imho.

@sebastianblum
Copy link
Contributor

sebastianblum commented Jan 2, 2018

@dbu can you squash #1029 ? is squashing commits while merging enabled for this repo?

@dbu
Copy link
Member Author

dbu commented Jan 3, 2018

fixed in #1029

@dbu dbu closed this Jan 3, 2018
@dbu dbu deleted the fix-build branch January 3, 2018 07:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
State: Reviewing This item is being reviewed to determine if it should be accepted.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants