Skip to content

Fix testExtensionsAreNotInitializedWhenRenderingACompiledTemplate#4263

Merged
fabpot merged 1 commit into
twigphp:3.xfrom
derrabus:improvement/init-extensions-test
Sep 2, 2024
Merged

Fix testExtensionsAreNotInitializedWhenRenderingACompiledTemplate#4263
fabpot merged 1 commit into
twigphp:3.xfrom
derrabus:improvement/init-extensions-test

Conversation

@derrabus

@derrabus derrabus commented Sep 2, 2024

Copy link
Copy Markdown
Contributor

I stumbled over this test while checking for PHPUnit deprecations. In this case, we need to do something about the setMethods() call which has been removed in PHPUnit 11.

However, we add an expectation for a method initExtensions() which has been removed a long time ago. So, if that method was actually called, we'd get an error anyway. Expecting this method not to be called feels a little redundant. This basically means, we can run this test against an unmocked Environment. But if we do so, the test does not really test what it's supposed to test which is why I decided to remove the whole test.

Update: Instead, I'm now registering an extension that throws if we try to initialize it.

@stof

stof commented Sep 2, 2024

Copy link
Copy Markdown
Member

you could check that getFunctions or getFilters is not called on a registered extension as a way to detect that the initialization is not performed (which also does not require mocking the Environment AFAIK).

@derrabus

derrabus commented Sep 2, 2024

Copy link
Copy Markdown
Contributor Author

I tried your suggestion:

$twig = new Environment($loader, $options);
$extension = $this->createMock(ExtensionInterface::class);
$extension->expects($this->never())->method($this->anything());
$twig->addExtension($extension);

With this, the test fails. My assumption is that the test guesses the cache key wrong.

@derrabus

derrabus commented Sep 2, 2024

Copy link
Copy Markdown
Contributor Author

Adding the extension changes the cache key. 🤦🏻‍♂️ I'll try to work around that.

@derrabus derrabus force-pushed the improvement/init-extensions-test branch from 2fcd652 to 3a4d513 Compare September 2, 2024 18:07
@derrabus

derrabus commented Sep 2, 2024

Copy link
Copy Markdown
Contributor Author

I think, I've fixed the test. Thank you for your help, @stof.

@derrabus derrabus changed the title Remove obsolete test Fix testExtensionsAreNotInitializedWhenRenderingACompiledTemplate Sep 2, 2024
@fabpot

fabpot commented Sep 2, 2024

Copy link
Copy Markdown
Contributor

Thank you @derrabus.

@fabpot fabpot merged commit c0f1154 into twigphp:3.x Sep 2, 2024
@derrabus derrabus deleted the improvement/init-extensions-test branch September 2, 2024 20:39
@derrabus derrabus mentioned this pull request Sep 3, 2024
10 tasks
fabpot added a commit that referenced this pull request Sep 4, 2024
This PR was squashed before being merged into the 4.x branch.

Discussion
----------

PHPUnit 11

This PR switches our CI to PHPUnit 11.

TODO:

- [x] #4262
- [x] #4263
- [x] #4264
- [x] #4265
- [x] #4268
- [x] #4266
- [x] #4270
- [x] #4271
- [x] #4272
- [x] #4274

Commits
-------

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants