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

[Yii2] Fixed `@webroot` directory creation #3387

Merged
merged 1 commit into from Jul 29, 2016

Conversation

Projects
None yet
4 participants
@DavertMik
Member

DavertMik commented Jul 28, 2016

Proposed by @dmitry-kulikov

@DavertMik DavertMik referenced this pull request Jul 28, 2016

Closed

Yii2 `@webroot` fix #3381

@samdark samdark added the Yii label Jul 29, 2016

@samdark

This comment has been minimized.

Show comment
Hide comment
@samdark

samdark Jul 29, 2016

Collaborator

Looks good to me.

Collaborator

samdark commented Jul 29, 2016

Looks good to me.

@DavertMik DavertMik merged commit 0f74d9b into 2.2 Jul 29, 2016

5 checks passed

continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
semaphoreci The build passed on Semaphore.
Details

@DavertMik DavertMik deleted the yii-webroot-fix branch Jul 29, 2016

@githubjeka

This comment has been minimized.

Show comment
Hide comment
@githubjeka

githubjeka Nov 9, 2016

Contributor

Don't work for me. :(

Codeception version 2.2.6

Contributor

githubjeka commented Nov 9, 2016

Don't work for me. :(

Codeception version 2.2.6

@githubjeka

This comment has been minimized.

Show comment
Hide comment
@githubjeka

githubjeka Nov 9, 2016

Contributor

If use command $I->click('save-user'); @webroot will be created...

Contributor

githubjeka commented Nov 9, 2016

If use command $I->click('save-user'); @webroot will be created...

@githubjeka

This comment has been minimized.

Show comment
Hide comment
@githubjeka

githubjeka Nov 9, 2016

Contributor

The code return $this->redirect(['view', 'id' => $user->id]); will run after click ...
Replace to return $this->render('view', ['model' => $user,]); and @webroot doesn't created.
@samdark What could be the problem?

Contributor

githubjeka commented Nov 9, 2016

The code return $this->redirect(['view', 'id' => $user->id]); will run after click ...
Replace to return $this->render('view', ['model' => $user,]); and @webroot doesn't created.
@samdark What could be the problem?

@samdark

This comment has been minimized.

Show comment
Hide comment
@samdark

samdark Nov 9, 2016

Collaborator

No idea.

Collaborator

samdark commented Nov 9, 2016

No idea.

@githubjeka

This comment has been minimized.

Show comment
Hide comment
@githubjeka

githubjeka Nov 9, 2016

Contributor

After redirect on page uses:

$this->registerJsFile('@web/js/d3Temperature.js',
    ['depends' => [\yii\web\JqueryAsset::className(), \common\assets\D3Asset::className()]]);
public function registerJsFile($url, $options = [], $key = null)
    {
        $url = Yii::getAlias($url);
        $key = $key ?: $url;

        $depends = ArrayHelper::remove($options, 'depends', []);

        if (empty($depends)) {
            $position = ArrayHelper::remove($options, 'position', self::POS_END);
            $this->jsFiles[$position][$key] = Html::jsFile($url, $options);
        } else {
            $this->getAssetManager()->bundles[$key] = Yii::createObject([
                'class' => AssetBundle::className(),
                'baseUrl' => '',
                'js' => [strncmp($url, '//', 2) === 0 ? $url : ltrim($url, '/')],
                'jsOptions' => $options,
                'depends' => (array)$depends,
            ]);
            $this->registerAssetBundle($key);
        }
    }

P.S. How use XDebug with codecept ?

Contributor

githubjeka commented Nov 9, 2016

After redirect on page uses:

$this->registerJsFile('@web/js/d3Temperature.js',
    ['depends' => [\yii\web\JqueryAsset::className(), \common\assets\D3Asset::className()]]);
public function registerJsFile($url, $options = [], $key = null)
    {
        $url = Yii::getAlias($url);
        $key = $key ?: $url;

        $depends = ArrayHelper::remove($options, 'depends', []);

        if (empty($depends)) {
            $position = ArrayHelper::remove($options, 'position', self::POS_END);
            $this->jsFiles[$position][$key] = Html::jsFile($url, $options);
        } else {
            $this->getAssetManager()->bundles[$key] = Yii::createObject([
                'class' => AssetBundle::className(),
                'baseUrl' => '',
                'js' => [strncmp($url, '//', 2) === 0 ? $url : ltrim($url, '/')],
                'jsOptions' => $options,
                'depends' => (array)$depends,
            ]);
            $this->registerAssetBundle($key);
        }
    }

P.S. How use XDebug with codecept ?

@githubjeka

This comment has been minimized.

Show comment
Hide comment
@githubjeka

githubjeka Nov 9, 2016

Contributor

@samdark
this PR:

$this->app->set('assetManager', Stub::make('yii\web\AssetManager', ['bundles' => false]));

To fix my issue need:

$this->app->set('assetManager', Stub::make('yii\web\AssetManager', ['bundles' => false, 'publish'=>function(){}]));
Contributor

githubjeka commented Nov 9, 2016

@samdark
this PR:

$this->app->set('assetManager', Stub::make('yii\web\AssetManager', ['bundles' => false]));

To fix my issue need:

$this->app->set('assetManager', Stub::make('yii\web\AssetManager', ['bundles' => false, 'publish'=>function(){}]));

@githubjeka githubjeka referenced this pull request Nov 9, 2016

Closed

Yii2: @webroot dir #3706

@lukascernydis

This comment has been minimized.

Show comment
Hide comment
@lukascernydis

lukascernydis Nov 14, 2016

Hi, I have discovered the reason why @webroot directory is created.
When AssetManager is mocked, it's basePath and baseUrl is set with an alias, that don't get translated.

 basePath = "@webroot/assets"
 baseUrl = "@web/assets"

The following change in codeception Yii connector fixed all my issues
codeception/base/src/Codeception/Lib/Connector/Yii2.php

private function mockAssetManager()
{
    $this->app->set('assetManager', Stub::make('yii\web\AssetManager', ['bundles' => false]));
    $this->app->getAssetManager()->basePath = Yii::getAlias($this->app->getAssetManager()->basePath);
    $this->app->getAssetManager()->baseUrl = Yii::getAlias($this->app->getAssetManager()->baseUrl);
}

@samdark Is this fix ok?

lukascernydis commented Nov 14, 2016

Hi, I have discovered the reason why @webroot directory is created.
When AssetManager is mocked, it's basePath and baseUrl is set with an alias, that don't get translated.

 basePath = "@webroot/assets"
 baseUrl = "@web/assets"

The following change in codeception Yii connector fixed all my issues
codeception/base/src/Codeception/Lib/Connector/Yii2.php

private function mockAssetManager()
{
    $this->app->set('assetManager', Stub::make('yii\web\AssetManager', ['bundles' => false]));
    $this->app->getAssetManager()->basePath = Yii::getAlias($this->app->getAssetManager()->basePath);
    $this->app->getAssetManager()->baseUrl = Yii::getAlias($this->app->getAssetManager()->baseUrl);
}

@samdark Is this fix ok?

@githubjeka

This comment has been minimized.

Show comment
Hide comment
@githubjeka

githubjeka Nov 15, 2016

Contributor

No, hasn't fix the issue #3706
In Yii2 tests @webroot is root codeception.yml location

Contributor

githubjeka commented Nov 15, 2016

No, hasn't fix the issue #3706
In Yii2 tests @webroot is root codeception.yml location

@lukascernydis

This comment has been minimized.

Show comment
Hide comment
@lukascernydis

lukascernydis Nov 15, 2016

How about this? bundles => false is not necessary anymore.

private function mockAssetManager()
{
    $this->app->set('assetManager', Stub::make('yii\web\AssetManager'));
    $this->app->getAssetManager()->basePath = realpath(Yii::getAlias($this->app->getAssetManager()->basePath));
    $this->app->getAssetManager()->baseUrl = Yii::getAlias($this->app->getAssetManager()->baseUrl);
}

I have tried with this and works like a charm

$this->registerJsFile('@web/js/iCheck.js', ['depends' => [
    \yii\web\JqueryAsset::className(), 
    \yii\bootstrap\BootstrapAsset::className()
]]);

realpath() usage is to translate to absolute path to fix my another problem with SCSS convereter error

lukascernydis commented Nov 15, 2016

How about this? bundles => false is not necessary anymore.

private function mockAssetManager()
{
    $this->app->set('assetManager', Stub::make('yii\web\AssetManager'));
    $this->app->getAssetManager()->basePath = realpath(Yii::getAlias($this->app->getAssetManager()->basePath));
    $this->app->getAssetManager()->baseUrl = Yii::getAlias($this->app->getAssetManager()->baseUrl);
}

I have tried with this and works like a charm

$this->registerJsFile('@web/js/iCheck.js', ['depends' => [
    \yii\web\JqueryAsset::className(), 
    \yii\bootstrap\BootstrapAsset::className()
]]);

realpath() usage is to translate to absolute path to fix my another problem with SCSS convereter error

@githubjeka

This comment has been minimized.

Show comment
Hide comment
@githubjeka

githubjeka Nov 15, 2016

Contributor

With your code:
image

In Yii2 tests @webroot is root codeception.yml location

Correct fix is setting basePath & baseUrl of assetManager in your confguration of Application
See yiisoft/yii2-app-advanced#219 , for example.

Contributor

githubjeka commented Nov 15, 2016

With your code:
image

In Yii2 tests @webroot is root codeception.yml location

Correct fix is setting basePath & baseUrl of assetManager in your confguration of Application
See yiisoft/yii2-app-advanced#219 , for example.

@lukascernydis

This comment has been minimized.

Show comment
Hide comment
@lukascernydis

lukascernydis Nov 15, 2016

With basePath and baseUrl set in config, settings are correct until this code occurs:

$this->app->set('assetManager', Stub::make('yii\web\AssetManager'));

before:
basePath = "C:\web\domains\yii.dev\config/../assets"

after:

basePath = "@webroot/assets"
baseUrl = "@web/assets"

This aliases will never get translated.

I am currently using basic template with codeception.yml in @webroot.
I will try to fix it on advanced template too.

lukascernydis commented Nov 15, 2016

With basePath and baseUrl set in config, settings are correct until this code occurs:

$this->app->set('assetManager', Stub::make('yii\web\AssetManager'));

before:
basePath = "C:\web\domains\yii.dev\config/../assets"

after:

basePath = "@webroot/assets"
baseUrl = "@web/assets"

This aliases will never get translated.

I am currently using basic template with codeception.yml in @webroot.
I will try to fix it on advanced template too.

@githubjeka

This comment has been minimized.

Show comment
Hide comment
@githubjeka

githubjeka Nov 15, 2016

Contributor

see #3742

Contributor

githubjeka commented Nov 15, 2016

see #3742

@lukascernydis

This comment has been minimized.

Show comment
Hide comment
@lukascernydis

lukascernydis Nov 15, 2016

Yes, this is definitely the solution.

I was hoping for some global codeception fix so will avoid to overriding codeception files locally.

lukascernydis commented Nov 15, 2016

Yes, this is definitely the solution.

I was hoping for some global codeception fix so will avoid to overriding codeception files locally.

@githubjeka

This comment has been minimized.

Show comment
Hide comment
@githubjeka

githubjeka Nov 15, 2016

Contributor

I tried to search this fix too. If you will find - welcome to PR.

Contributor

githubjeka commented Nov 15, 2016

I tried to search this fix too. If you will find - welcome to PR.

@githubjeka

This comment has been minimized.

Show comment
Hide comment
@githubjeka
Contributor

githubjeka commented Nov 15, 2016

@lukascernydis

This comment has been minimized.

Show comment
Hide comment
@lukascernydis

lukascernydis Nov 15, 2016

I will try to fix it in both templates in the evening. All fixes that I found are only curing the symptoms - not solving the issue.

Why do we actually need to mock the asset manager?

Thanks for your effort :-)

lukascernydis commented Nov 15, 2016

I will try to fix it in both templates in the evening. All fixes that I found are only curing the symptoms - not solving the issue.

Why do we actually need to mock the asset manager?

Thanks for your effort :-)

@githubjeka

This comment has been minimized.

Show comment
Hide comment
@githubjeka

githubjeka Nov 15, 2016

Contributor

Why do we actually need to mock the asset manager?

A first functional test is slow for fresh application, because assertManager will generate files, which in functional case not used (#3706 (comment))

Contributor

githubjeka commented Nov 15, 2016

Why do we actually need to mock the asset manager?

A first functional test is slow for fresh application, because assertManager will generate files, which in functional case not used (#3706 (comment))

@lukascernydis

This comment has been minimized.

Show comment
Hide comment
@lukascernydis

lukascernydis Nov 15, 2016

How about this? All bundles being registered are replaced with empty bundle so nothing get published.

private function mockAssetManager()
{
    $this->app->set('view', Stub::make('yii\web\View', [
        'registerAssetBundle' => function() {
            return new AssetBundle();
        }
    ]));
    $this->app->set('assetManager', Stub::make('yii\web\AssetManager', ['bundles' => false]));
}

lukascernydis commented Nov 15, 2016

How about this? All bundles being registered are replaced with empty bundle so nothing get published.

private function mockAssetManager()
{
    $this->app->set('view', Stub::make('yii\web\View', [
        'registerAssetBundle' => function() {
            return new AssetBundle();
        }
    ]));
    $this->app->set('assetManager', Stub::make('yii\web\AssetManager', ['bundles' => false]));
}
@githubjeka

This comment has been minimized.

Show comment
Hide comment
@githubjeka

githubjeka Nov 15, 2016

Contributor

If you mock component then component can't be configurate from config. AssetManager can be mocked for this case, but View - no, I don't think.

Contributor

githubjeka commented Nov 15, 2016

If you mock component then component can't be configurate from config. AssetManager can be mocked for this case, but View - no, I don't think.

@lukascernydis

This comment has been minimized.

Show comment
Hide comment
@lukascernydis

lukascernydis Nov 15, 2016

Hope this will be the last try. Empty bundles like in previous attempt, but now in AssetManager. Works with dependencies as well.

private function mockAssetManager()
{
    $this->app->set('assetManager', Stub::make('yii\web\AssetManager', [
        'bundles' => false,
        'getBundle' => function () {
            return Yii::createObject('yii\web\AssetBundle');
        },
    ]));
}

lukascernydis commented Nov 15, 2016

Hope this will be the last try. Empty bundles like in previous attempt, but now in AssetManager. Works with dependencies as well.

private function mockAssetManager()
{
    $this->app->set('assetManager', Stub::make('yii\web\AssetManager', [
        'bundles' => false,
        'getBundle' => function () {
            return Yii::createObject('yii\web\AssetBundle');
        },
    ]));
}
@githubjeka

This comment has been minimized.

Show comment
Hide comment
@githubjeka

githubjeka Nov 16, 2016

Contributor

This broked my tests by

[yii\base\UnknownMethodException] Calling unknown method: yii\web\AssetBundle::addLanguage()

Contributor

githubjeka commented Nov 16, 2016

This broked my tests by

[yii\base\UnknownMethodException] Calling unknown method: yii\web\AssetBundle::addLanguage()

@lukascernydis

This comment has been minimized.

Show comment
Hide comment
@lukascernydis

lukascernydis Nov 16, 2016

This should fix error when using custom AssetBundles

private function mockAssetManager()
{
    $this->app->set('assetManager', Stub::make('yii\web\AssetManager', [
        'bundles' => false,
        'getBundle' => function ($name) {
            if (!class_exists($name)) {
                $name = 'yii\web\AssetBundle';
            }

            return Yii::createObject($name);
        },
    ]));
}

lukascernydis commented Nov 16, 2016

This should fix error when using custom AssetBundles

private function mockAssetManager()
{
    $this->app->set('assetManager', Stub::make('yii\web\AssetManager', [
        'bundles' => false,
        'getBundle' => function ($name) {
            if (!class_exists($name)) {
                $name = 'yii\web\AssetBundle';
            }

            return Yii::createObject($name);
        },
    ]));
}
@githubjeka

This comment has been minimized.

Show comment
Hide comment
@githubjeka

githubjeka Nov 16, 2016

Contributor

I have the code

$this->assetManager->publish('@common/assets/network-data-save/readout.js');

and your code provide this:
image

Contributor

githubjeka commented Nov 16, 2016

I have the code

$this->assetManager->publish('@common/assets/network-data-save/readout.js');

and your code provide this:
image

@lukascernydis

This comment has been minimized.

Show comment
Hide comment
@lukascernydis

lukascernydis Nov 16, 2016

private function mockAssetManager()
{
    $this->app->set('assetManager', Stub::make('yii\web\AssetManager', [
        'bundles' => false,
        'getBundle' => function ($name) {
            if (!class_exists($name)) {
                $name = 'yii\web\AssetBundle';
            }

            return Yii::createObject($name);
        },
        'publish' => function () {
            return [null, null];
        },
    ]));
}

lukascernydis commented Nov 16, 2016

private function mockAssetManager()
{
    $this->app->set('assetManager', Stub::make('yii\web\AssetManager', [
        'bundles' => false,
        'getBundle' => function ($name) {
            if (!class_exists($name)) {
                $name = 'yii\web\AssetBundle';
            }

            return Yii::createObject($name);
        },
        'publish' => function () {
            return [null, null];
        },
    ]));
}
@githubjeka

This comment has been minimized.

Show comment
Hide comment
@githubjeka

githubjeka Nov 16, 2016

Contributor

Like as #3387 (comment)

Result - #3719 (comment)

To fix you should create AssetManagerInterface and use his implement in your tests. No in a tool for tests.
Creating @webroot directory not issue of Codeception, it issue of Yii2. And should be fix in framework, otherwise in your code. Like as now via config.

Contributor

githubjeka commented Nov 16, 2016

Like as #3387 (comment)

Result - #3719 (comment)

To fix you should create AssetManagerInterface and use his implement in your tests. No in a tool for tests.
Creating @webroot directory not issue of Codeception, it issue of Yii2. And should be fix in framework, otherwise in your code. Like as now via config.

@lukascernydis

This comment has been minimized.

Show comment
Hide comment
@lukascernydis

lukascernydis Nov 16, 2016

It is not possible to override AssetManager trough Yii config when mockAssetManager() replaces it with yii\web\AssetManager again.

We should respect AssetManager class set in config. So the mock will look like:

private function mockAssetManager()
{
    $this->app->set('assetManager', Stub::make(get_class($this->app->getAssetManager()), ['bundles' => false]));
}

lukascernydis commented Nov 16, 2016

It is not possible to override AssetManager trough Yii config when mockAssetManager() replaces it with yii\web\AssetManager again.

We should respect AssetManager class set in config. So the mock will look like:

private function mockAssetManager()
{
    $this->app->set('assetManager', Stub::make(get_class($this->app->getAssetManager()), ['bundles' => false]));
}
@githubjeka

This comment has been minimized.

Show comment
Hide comment
@githubjeka

githubjeka Nov 16, 2016

Contributor

That why appeared this PR

Users should get to options between confrig or mock in his application. By default it's config option.

Contributor

githubjeka commented Nov 16, 2016

That why appeared this PR

Users should get to options between confrig or mock in his application. By default it's config option.

@lukascernydis

This comment has been minimized.

Show comment
Hide comment
@lukascernydis

lukascernydis Nov 16, 2016

#3742 sounds good.

Thank you for patience.

lukascernydis commented Nov 16, 2016

#3742 sounds good.

Thank you for patience.

@githubjeka

This comment has been minimized.

Show comment
Hide comment
@githubjeka

githubjeka Nov 16, 2016

Contributor

Thank you too.

Contributor

githubjeka commented Nov 16, 2016

Thank you too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment