Cannot mock classes marked final

[ol1s]

@lcobucci I know that this is 100% intentional and that you probably have many reasons why you do not want others to extend your classes 😁

But......does every class really need to be marked as final !!!

This is especially painful when trying to mock these classes in my unit tests as I get the following error from PHPUnit:

The class is marked final and its methods cannot be replaced

Any chance you could consider removing the final declarations?

[lcobucci]

@ol1s making my classes final is the best way to make my design choices explicit.

If I leave the class open for inheritance, it becomes an extension point and people may rely on its internals do do other things. That also means that if I decide to make chances to how these classes work, it will be a BC-break.
I don't want that to happen because it creates more work for me in the end. So, no, I won't consider removing final from classes.

The extension points in the library are explicitly defined with interfaces and you can surely mock these interfaces. The question is, though: when should you mock them? Mocking is great until it becomes extremely painful =)

[ol1s]

Thanks for the reply, it's understood 😁

Just as a reference as to what I was originally planning.....here is a rough example of what I was hoping to do - basically shows mocking the lcobucci/jwt classes, meaning that I can isolate this code away from the code I am actually testing (oh and means that I don't need any test keys setup either!)

//phpunit test code using mockery
$bearerToken = 'TestToken123'; //doesnt need to be a real token in this test
$constraints = []; //doesnt need real constraints in this test
$tokenConfig = Mockery::mock(Configuration::class);
$token = Mockery::mock(UnencryptedToken::class);
$tokenConfig->shouldReceive('parser->parse')->once()->with($bearerToken)->andReturn($token);
$tokenConfig->shouldReceive('validationConstraints')->once()->andReturn($constraints);
$tokenConfig->shouldReceive('validator->validate')->once()->with($token, $constraints)->andReturn(true);
$token->shouldReceive('claims->has')->once()->with('sub')->andReturn(true);
$token->shouldReceive('claims->get')->once()->with('sub')->andReturn(123456);

I guess what you're saying is I shouldn't really be trying to isolate this package code from where it is being used - instead test them as a whole.

[lcobucci]

@ol1s your approach here is to verify method calls, which works fine but I honestly don't think it's a good choice for this particular case as your tests:

1. become extremely verbose
2. become might break easily due to teeny tiny changes (on the lib or your codebase)
3. aren't testing your software behaviour (only checking method invocation against a fake implementation)

I think that test doubles are useful sometimes but if you ask my good friend @frankdejonge, he'll provide very good reasons to think twice before using them: https://blog.frankdejonge.nl/testing-without-mocking-frameworks/

In this particular case, I'd suggest using something like HS256 on your tests, which won't bring any negative impact on the performance of your test suite and will mitigate the points above.

For example, testing what happens when the token's signature doesn't match your expectations can look like this:

$config = Configuration::forSymmetricSigner(new Sha256(), InMemory::plainText('my-correct-key'));
$config->setValidationConstraints(new SignedWith($config->signer(), $config->verificationKey()));

$bearerToken = $config->builder()
    ->relatedTo(123456)
    ->getToken($config->signer(), InMemory::plainText('not-my-key'))
    ->toString();

$this->expectException(SomeException::class);
$this->expectExceptionMessage('Computer says no');
$callYourSystemToParseAndValidateTheToken($bearerToken);

[ol1s]

Thanks @lcobucci for your helpful insight and guidance.

I feel like it's interesting times, the community seems like it is moving away from the "unit" testing ideology where every element of a system is isolated and tested individually. I have experienced this in the JS community as well as PHP.

Interestingly, Laravel framework also now appears to be advocating "feature" tests as opposed to unit tests (they have started pushing these a lot harder in latest version of their docs). I guess these "feature" tests align more with what you have described above - leaving the mockist approach behind and instead booting the application and testing the system as a whole.

[lcobucci]

@ol1s I think the problem is that the definition of a "unit" is something not clearly defined for everyone. Then, we end up creating tests which give us more work and less value.

Fowler has an interesting article on the topic: https://martinfowler.com/articles/2021-test-shapes.html