[RFC] Make Hamcrest A Required Dependency

[padraic]

Should end the debate over adding more matchers directly to Mockery, and also promote using the Hamcrest ones more consistently since everyone should now have it installed.

This assumes there are no other options we should be considering. Are there?

[aik099]

Yeah, too many guys I think I have a matcher idea for Mockery tickets lately. Maybe we even can:

• remove Mockery's own matchers (that would be a BC break for sure)
• use Hamcrest matchers inside Mockery matcher classes (or extend from them)

[padraic]

I'd be up for using them internal to our own API. I think the BC break might be too far for 1.0.0 though clearly I should not have run afoul of NIH syndrome to start with!

[padraic]

We're unlikely to change anything prior to 1.0.0, if even then so suddenly, so unless there are any objections, I'll commit it as a require tomorrow.

[padraic]

Assuming no objections, I'll commit this change later.

[JanTvrdik]

👎 This makes Mockery installation a lot heavier.

[gmazzap]

👎 Since this has been introducued all my test fails because of #464

Mockery is perfectly functional without Hamcrest, so Hamcrest is not a dependency. Why force it to be?

[GrahamCampbell]

@Giuseppe-Mazzapica #464 has already been fixed.

[gmazzap]

@GrahamCampbell where? If I require via Composer Mockery 0.9.4 (last one) and last PHPunit and I manually load all the PHP functions file during PHPUnit bootstrap (I need that because my tests uses PHPUnit functions) I still get the "function already defined" error.

[GrahamCampbell]

The very latest releases of phpunit and hamcrest should work.

[GrahamCampbell]

The hamcrest release only came out yesterday mind you. ;)

[gmazzap]

@GrahamCampbell I don't require hamcrest, because I don't use it. Mockery 0.9.4 requires it for me.

If I composer-require

• PHPUnit 4.6.6 (last stable)
• Mockery 0.9.4 (last)

and I do (in the PHPUnit bootstrap file):

require_once 'path/to/vendor/autoload.php';
require_once  'path/to/vendor/phpunit/phpunit/src/Framework/Assert/Functions.php';

When I run tests I get:

Fatal error: Cannot redeclare any() (previously declared in ...) in ...

On the countrary, if I composer-require

• PHPUnit 4.6.6 (last stable)
• Mockery 0.9.3 (previous)

Everything works well.

[davedevelopment]

@Giuseppe-Mazzapica

try

composer update hamcrest/hamcrest-php

then reverse the order of those includes

require_once  'path/to/vendor/phpunit/phpunit/src/Framework/Assert/Functions.php';
require_once 'path/to/vendor/autoload.php';

[gmazzap]

Thanks @davedevelopment

try composer update hamcrest/hamcrest-php

If this is the solution, it is not the case to release a 0.9.5 of Mockery that requires last version of hamcrest? As already said, I dont require hamcrest, Mockery does it for me.

[davedevelopment]

@Giuseppe-Mazzapica yes and no. You'd still have the problem if you didn't reverse the order of those includes (I think), which we can't do anything about. PHPUnit dumps those functions in the global namespace without consideration for anything that might already exist.

[gmazzap]

@davedevelopment

Ok, actually 0.9.4 version of Mockery already requires the last version of hamcrest.

The solution was just to reverse the order of includes.

My opition:

• this is a pretty fragile solution that depends on the order that consumers requires 3rd party things
• there was no need to make hamcrest a required dependency for Mockery, because it is not a required dependency: Mockery works perfectly without it

The latter point was the reason I decided to comment here instead on #464 ...

[padraic]

Closing this as it's done and I don't want to confuse issues with old ones.

While recognising there is fragility, we've mitigated it as best we can short of reversing the RFC, and I'm not leaning towards a reversal. Mockery only has basic matchers, and we rely on Hamcrest to extend those. Most of the fragility isn't ours, and could be solved by PHPUnit putting some basic checks in place before loading functions into the global namespace.

If it remains a concern, please feel free to bring up in a separate issue.