Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.I noticed that Drupal\rest\Plugin\Type\ResourcePluginManager has no mechanism for dealing with #1780396: Namespaces of disabled modules are registered during test runs, which indicated that it didn't have any test plugins.
This means that we don't really have test coverage for what a resource plugin should do.
Comments
Comment #1
klausiAt the very least a resource plugin MUST implement the routes() and permissions() methods as well as PluginInspectionInterface. I think we should introduce a ResourceInterface for this, currently it is implicit behaviour expected of a plugin.
A resource plugin SHOULD extend ResourceBase and it will get a lot of logic for free.
When using ResourceBase a plugin MAY implement HTTP methods as PHP methods such as get(), post(), put() etc.
Not sure why we should have a test plugin? I can assume that the plugin system works and is tested elsewhere? And when we create ResourceInterface it is clear what a plugin should do?
Comment #2
tim.plunkettAn implementation of the interface (once #1874820: Resource plugins miss an interface is in) should be explicitly unit tested, not just implicitly tested elsewhere.
Comment #3
tim.plunkettWe still don't have any tests for this, PHPUnit would be nice here.
Comment #4
clemens.tolboomHow should we write those tests? I cannot find a PluginTest example yet.
modules/tour/src/Tests/TourPluginTest.php does extend KernelTestBase which is not PHPUnit
modules/rest/src/Tests/Views/StyleSerializerTest.php seemed better but is complete views dependent.
Comment #5
Wim LeersComment #6
Wim LeersComment #7
afi13 CreditAttribution: afi13 commentedI am on it.
Comment #8
zpul CreditAttribution: zpul as a volunteer and at MontenaSoft commentedhi, @afi13: I would like to try this issue... do you have any updates so far?
Comment #9
afi13 CreditAttribution: afi13 commentedhi, @zpul, it's good idea, because i have no time for now. You can take it.
Comment #10
afi13 CreditAttribution: afi13 commentedComment #11
zpul CreditAttribution: zpul as a volunteer and at MontenaSoft commented@afi13: thanx, assigned to me
Comment #12
tstoeckler@zpul: Are you still working on this?
Tentatively tagging as #DrupalBCDays to keep it on the radar for someone at the sprint on the weekend...
Comment #13
zpul CreditAttribution: zpul as a volunteer and at MontenaSoft commented@tstoeckler: hi, need to drop on this issue, too much work pressure currently
Comment #15
tetranz CreditAttribution: tetranz as a volunteer commentedI'm happy to have a go at this but I need some clarification about what we need.
I can see how to build a resource plugin and write some tests for it.
Don't we effectively already have this with the EntityResource plugin? It is part of the rest module and has some tests.
I guess I'm misunderstanding what's missing.
Comment #17
mradcliffeCleaning up issue tags.
Comment #18
Wim Leers#2737719: EntityResource: Provide comprehensive test coverage: for every entity type, every format, every method added explicit test coverage for the
EntityResourceplugin implementation.Retitling per @tim.plunkett in #2 and before.
Comment #19
tim.plunkettThis may have become obsolete after #1874820: Resource plugins miss an interface
When I opened this there was no \Drupal\rest\Plugin\ResourceInterface, so no way to know which methods were available or what they did.
I noticed that there was no coverage of ResourcePluginManager, because there had been a bug that affected all subclasses of DefaultPluginManager, and only RPM didn't fail.
Looking at ResourcePluginManager now, and knowing how much more robust DefaultPluginManager is, and since the only method (ResourcePluginManager::getInstance()) is now deprecated, I think we can close this.