[Bug] mix() function throw exceptions during testing.

[mathieutu]

Hi, as @themsaid asked in https://github.com/laravel/framework/issues/19552, I'm reposting here :

I have a problem with the mix() function. When I'm executing my tests, especially with travis I don't run all the webpack stuff: I don't need it, as I'm just testing controllers, services, etc.

Not Js and css things.

BUT, when I'm trying to make a get on a route which return a view, I always have a an exception from the mix() helper method.

Example :

Route::get(/foo, function () { return view('bar'); }

$response = $this->get('/foo');
$response->assertViewIs('bar');

Will throw a The Mix manifest does not exist. exception.

This is really annoying to have to install modules and compile assets we don't need..!

And we can't overwrite, or mock, or anything, because this is not part of the container or something like that, but just a basic function in helpers.php file.

Maybe we can make a service for that, and just do:

function mix($path, $manifestDirectory = '')
{
    return app('mix')->mix($path, $manifestDirectory);
}

in helpers.php

It will permit to mock it in tests:

app()->bind('mix', function () {
    return Mockery::mock(Mix::class)->shouldReceive('mix')->andReturn('')->getMock();
});

or something like that.

I can make the PR when we have choose a solution.

For now, I've just made this bad thing to fix it:

// New function which is called instead of mix in my views...
function mix_except_in_tests($path, $manifestDirectory = '') {
    if  (app()->runningUnitTests()){
        return '';
    }

    return mix($path, $manifestDirectory);
}

This is also a (quickest and dirtiest) option, but it may cause conflict with Laravel dusk ?

We also could do sort of:

function mix($path, $manifestDirectory = '') {
    if  (app()->runningUnitTests() && config('should_not_compile_during_testing')) {
        return '';
    }

    ... // old mix function.
}

I'm waiting for all your good ideas guys !

Matt'

[joshmanders]

Just ran into this issue on CircleCI.

[sebastiandedeyne]

A while ago Taylor said he was interested in resolving this issue, so I think a PR would definitely be welcome.

I think I'd rather keep the mix function and not introduce another one.

Maybe a config value like mix.environments?

[JulienTant]

function mix($path, $manifestDirectory = '')
{
    return app('mix')->mix($path, $manifestDirectory);
}

This is the way to go in my opinion.

[mathieutu]

Sounds good for me @sebastiandedeyne!

So for you we have to let all this stuff just in the helper function? I think it could be a good idea to migrate that to a proper dedicated service.

edit: didn't see @JulienTant comment (and your +1 on it)

[sebastiandedeyne]

Changed my mind, actually prefer @JulienTant's idea 😄

[mathieutu]

So, first case I described? Just a container, and then we have to mock it?

Or we can mix both: migrate to a proper service, AND use a config variable, which is my opinion.

[sebastiandedeyne]

Yeah that would definitely be easier to use. Not sure about what the variable should be called though...

Something like require_in_environments is way longer that Laravel config keys generally are...

[mathieutu]

Yep, but I like mix.environments. It may oblige to have a file just for that, but why not?!

or maybe in view.php, so config('view.mix_environments') ?

I personally prefer the first case.

[JulienTant]

There's two cases where the mix() function is useful :

• You want to use HMR
• You want to use the manifest file

I can understand that you want to mock it for the tests when you don't want to run your assets compilation, but i'm not a supporter of having a configuration option to "bypass" the function when you decide to use it (instead of just calling the assets() helper).

If you have this special case when you want to change something or bypass for some reason, you can always overload mix in a ServiceProvider to use your own implementation.

[sebastiandedeyne]

Using a manifest file and not wanting to depend on assets in a CI environment seems like a common enough use case to have an "easy way out" imo, instead of having to maintain a custom service provider in your application.

[JulienTant]

You can simply mock the app('mix') in your tests then.

I'm quite against the configuration here because you add in the code complexity which has the only purpose of helping you doing tests. Using app('mix') helps you decouple and allows you to overload / mock. Using a 'switch' variable feels a bit crappy to me IF the only reason is tests.

[mathieutu]

@JulienTant As you can see in my post, it was my first idea. But in fine, if we are a lot to have this need, it can be cool to do something easier.

If it's not a config variable, it totally can be just method which mock mix(), like Mix::mock() or something like that.

The only problem here is dusk, so the real question is:

Is there a reason why for the same test we want or not compile assets depending of the environment?

I don't think so personally, so this solution is totally ok for me.

[joshmanders]

Instead of resolving stuff out of the container, why not just check to see if mix-manifest.json exists, if it does cache it and return from that, otherwise just return the value requested.

[mathieutu]

@joshmanders because in case of a non testing env, mix() should trow an error if the manifest doesn't exist. The is the case now and it should do that actually.

[mathieutu]

@sebastiandedeyne @JulienTant Here we are 😃 ! I'm waiting for your reviews!

[mathieutu]

Guys, I'm sorry but @taylorotwell closed all the PRs I've made for that, without any consideration for all the comments, reviews, and after 3 weeks of discussions.

I've no idea of what is the problem, code is cleaner, fully tested, and all the demanded features was added.

I'm done with that, I've lost enough time, if someone want to try, please do !

The last PR is here : https://github.com/laravel/framework/pull/19895

[brunogaspar]

Thanks for your work @mathieutu and please don't get discouraged, as @taylorotwell mentioned on https://github.com/laravel/framework/pull/19895#issuecomment-313861655 smaller improvements are always better than rewriting the whole thing in one go.

While i would really appreciate your PR to be merged i also see @taylorotwell point of view.

Maybe @taylorotwell can shed some light on what he would prefer to see done first?

[royduin]

Just ran into the same issue, thanks for your work @mathieutu! Using your helper function for the time being, hope @taylorotwell will fix this.

[ghost]

Yeah, thank you @mathieutu for working so hard on a fix for this, really disappointed this is still an issue.

[amylashley]

Just an observation: I too am having this issue. I did use the method that @mathieutu suggested: function mix($path, $manifestDirectory = '') { return app('mix')->mix($path, $manifestDirectory); } And although it works great with PHPUnit, it is causing my Dusk tests to fail.

[mathieutu]

@amylashley Can you give us some more information ? What did you do exactly, and what is the error ?

[amylashley]

Sure @mathieutu. I used this package to change the loading order so that I could add a helpers.php file. Then in my helpers.php file I added this method:

function mix($path, $manifestDirectory = '') {
    if  (app()->runningUnitTests()){
        return'';
    }

    return mix($path, $manifestDirectory);
}

This works great with PHPUnit. The test that was failing is now passing. However, with the same code in place, running Dusk (or Selenium with this package) both fail and give me only a white browser screen. That's the only feedback I seem to be able to get from Dusk- just a white screen. Once I comment out my new mix method, the Dusk tests are back to passing.

[mathieutu]

Hi, it's totally normal that in that case dusk fail, your function can't work properly. The mix() function is overwritten, so you can't call it inside itself. You're just doing some recursive calls...

[miken32]

This should be closed. You can simply run $this->withoutMix() in your test setup now. See https://github.com/laravel/framework/pull/30900