这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@ro0NL
Copy link
Contributor

@ro0NL ro0NL commented Jan 12, 2019

fixes #32

php bin/toolbox.php install 👍
bin/toolbox.php list-tools 👍
php bin/toolbox.php test 👍
php bin/devkit.php update:readme 👍

@ro0NL
Copy link
Contributor Author

ro0NL commented Jan 12, 2019

note the test is preceded by an auto-install of the wrapped phpunit, causing a bit of extra output: https://travis-ci.com/jakzal/toolbox/jobs/169989651#L1303

not sure how to avoid it, then again it's needed for testing 😅

@jakzal
Copy link
Owner

jakzal commented Jan 12, 2019

Hm... could we try sth like:

 "command": [
         "composer-bin-plugin": {
           "package": "symfony/phpunit-bridge",
           "namespace": "symfony"
         },
"sh": {
     "command": "simple-phpunit --version"
}
]

Not sure about exact syntax. I'd like to make sure all required files are installed on the image when we run this on docker.

@ro0NL
Copy link
Contributor Author

ro0NL commented Jan 12, 2019

the wrapped installation are not required files, those can vary depending on SYMFONY_PHPUNIT_VERSION env, as such multiple dists live in vendor/bin/.phpunit/phpunit-X.Y

it's a phpunit version manager basically :) so what about test -f vendor/bin/simple-phpunit for a test? and avoid invoking the binary (im not sure these files get included in the final dist at this point).

AFAIK there's no way to opt out from auto-installing. It's a wrapper, it's more or less required by design :) as such we should simply test the composer installation is OK.

@jakzal
Copy link
Owner

jakzal commented Jan 12, 2019

So how is it going to work on docker?

@ro0NL
Copy link
Contributor Author

ro0NL commented Jan 12, 2019

i've updated the test, it simply checks if the command exists:

/app # type simple-phpunit
simple-phpunit is /tools/.composer/vendor/bin/simple-phpunit
/app # echo $?
0
/app # type boo
boo: not found
/app # echo $?
127

let's see :)

edit: looks good 👌 https://travis-ci.com/jakzal/toolbox/jobs/170000687#L1274

@jakzal
Copy link
Owner

jakzal commented Jan 13, 2019

But what will happen once we install it on a docker image? Wouldn't each call trigger a phpunit installation?

@ro0NL
Copy link
Contributor Author

ro0NL commented Jan 13, 2019

Hi @jakzal, it depends ...

simple-phpunit will only install the wrapped phpunit on the first call, if it's not intalled yet. Possibly it can be done as an explicit step: simple-phpunit install. Once available it will forward everything to the wrapped phpunit binary.

If i understand well, we'll only lose it once the container/image gets destroyed/removed, triggering a re-install each time.

The wrapped binary can be installed in a custom location (ref), or by running a container with -e COMPOSER_HOME=/path, or using the proper volumes i suppose.

Currently i'm doing the same already by leveraging composer global require from the QA image.

@jakzal
Copy link
Owner

jakzal commented Jan 13, 2019

Possibly it can be done as an explicit step: simple-phpunit install.

That's why I was asking if something like the following helped:

 "command": [
         "composer-bin-plugin": {
           "package": "symfony/phpunit-bridge",
           "namespace": "symfony"
         },
    "sh": {
         "command": "simple-phpunit install"
    }
]

@ro0NL
Copy link
Contributor Author

ro0NL commented Jan 14, 2019

done. i think it's fine to pre-install the default version of the wrapped phpunit.

phpunit 7 support is added in current symfony master:
https://github.com/symfony/symfony/blob/master/src/Symfony/Bridge/PhpUnit/bin/simple-phpunit#L48

v.s. current stable 4.2.2 https://github.com/symfony/symfony/blob/v4.2.2/src/Symfony/Bridge/PhpUnit/bin/simple-phpunit#L48 which supports phpunit 6

as such we default install phpunit 6 still: https://travis-ci.com/jakzal/toolbox/jobs/170134793#L803

@jakzal
Copy link
Owner

jakzal commented Jan 14, 2019

Great job! I'll have a closer look asap.

@jakzal jakzal merged commit 6709922 into jakzal:master Jan 14, 2019
@ro0NL ro0NL deleted the simple-phpunit branch January 14, 2019 12:06
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.

Add Symfony PHPUnit bridge tool

2 participants