fixed node tests #1
Conversation
|
Well, this is already better: no error, but tests are failing. |
|
Comment copied from original issue Most tests are passing, see https://travis-ci.org/yiisoft/yii2-jquery/jobs/346996359 - I guess pjax can be removed and captcha also (move test to https://github.com/yiisoft/yii2-captcha) |
| - travis_retry composer install --prefer-dist --no-interaction | ||
| # setup PHP tests | ||
| - | | ||
| if [ $TASK_TESTS_PHP == 1 ]; then |
There was a problem hiding this comment.
I think we need that for JS tests as well...
There was a problem hiding this comment.
i.e. JS packages are installed via composer.
There was a problem hiding this comment.
Good point :) The question is how it will be handled in 2.1 ...
As laid out in https://github.com/yiisoft/yii2/issues/14862#issue-259192470 I think we should drop requirements in composer.json in favour of packages.json.
Since I've added it to packages.json, the npm install command fetches the packages needed for testing.
There was a problem hiding this comment.
@klimov-paul any ideas about it?
There was a problem hiding this comment.
How should I know?
It seems you and @schmunk42 have already decided to move towards new asset plugin. If you did, you should start to upgrade infrastructure, including the docs at the core and all asset-related extensions.
I would rather use CDN as default asset bundle source to aviod necessity of any plugin or other composer hack usage. This will allow particular extension to function out-of-the-box in most case, while leaving the developer choice in which way he want to install the assets.
I suppose it is too early to perform global asset management changes, like enforcing 'foxy' usage.
The core extensions yii2-gii and yii2-debug should be stripped from asset dependency first. Only after then this matter can be considered.
There was a problem hiding this comment.
|
Please remove PJAX and CAPTCHA from tests. |
|
I've removed the captcha part, but I'd suggest to do the PJAX stuff in a separate PR and merge this one for now. There are hundreds of |
|
Merged. Thanks. |
|
@samdark Side question, did you merge the commits squashed? Is this a (new) feature on GitHub or did you squash them manually? |
|
I did. It's not new. It's there for about 2 years. |
PR is for testing on Travis