-
Notifications
You must be signed in to change notification settings - Fork 5
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
Validate shortcode controllername service configurations (Case 173693) #40
Changes from 8 commits
ec8462b
277bddc
680eafa
ec8ad73
b666600
a9c04f6
8cbc736
3d62079
6f1bc2e
23024c8
cf8eea2
f29494d
640c445
86309e3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,6 @@ | |
|
||
namespace Webfactory\ShortcodeBundle\Tests\Functional; | ||
|
||
use InvalidArgumentException; | ||
use RuntimeException; | ||
use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; | ||
use Thunder\Shortcode\Handler\PlaceholderHandler; | ||
|
@@ -31,15 +30,6 @@ public function throws_exception_for_handlers_that_do_not_use_controllers(): voi | |
$this->helper->resolveShortcodeController('placeholder'); // uses the \Thunder\Shortcode\Handler\PlaceholderHandler handler class directly | ||
} | ||
|
||
/** | ||
* @test | ||
*/ | ||
public function throws_exception_for_shortcode_with_unresolvable_controller(): void | ||
{ | ||
self::expectException(InvalidArgumentException::class); | ||
$this->helper->resolveShortcodeController('test-config-invalid-controller'); | ||
} | ||
Comment on lines
-34
to
-41
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With this Test, the execution in an PHP8.1 and composer update --prefer-lowest environment failed, because the config for the shortcode named "test-config-invalid-controller" was loaded every time. Like in this run In this PR this should also be captured by the new test |
||
|
||
/** | ||
* @test | ||
*/ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure if I should also check for single
:
if I remember correctly, this was valid some time ago, too?In the docs https://www.php.net/manual/en/language.oop5.paamayim-nekudotayim.php there is no
:
alternateThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIK not in PHP, you might be thinking about Symfonys homebrew "bundle notation", which got deprecated in 4.1.
More importantly, I think we should allow invokable controllers as well. I'll add some changes shortly.