Problem/Motivation
There is no documentation for using the following tags.
- @coversDefaultClass
- @covers
- @group
See
- @covers http://phpunit.de/manual/current/en/appendixes.annotations.html#appendix...
- @coversDefaultClass http://phpunit.de/manual/current/en/appendixes.annotations.html#appendix...
- @group http://phpunit.de/manual/current/en/appendixes.annotations.html#appendix...
Benefits
If we adopted this change, the Drupal Project would benefit by ...
Three supporters required
- https://www.drupal.org/u/{userid} (yyyy-mm-dd they added support)
- https://www.drupal.org/u/{userid} (yyyy-mm-dd they added support)
- https://www.drupal.org/u/{userid} (yyyy-mm-dd they added support)
Proposed changes
Provide all proposed changes to the Drupal Coding standards. Give a link to each section that will be changed, and show the current text and proposed text as in the following layout:
1. {link to the documentation heading that is to change}
Current text
Add current text in blockquotes
Proposed text
Add proposed text in blockquotes
2. Repeat the above for each page or sub-page that needs to be changed.
Remaining tasks
Create this issue in the Coding Standards queue, using the defined template- Add supporters
- Create a Change Record
- Review by the Coding Standards Committee
- Coding Standards Committee takes action as required
- Discussed by the Core Committer Committee, if it impacts Drupal Core
- Final review by Coding Standards Committee
- Documentation updates
- Edit all pages
- Publish change record
- Remove 'Needs documentation edits' tag
- If applicable, create follow-up issues for PHPCS rules/sniffs changes
For a full explanation of these steps see the Coding Standards project page
Comments
Comment #1
dawehnerI really think we should not even try to write the same documentation as phpunit has already.
If someone wants to write a unit test, he will have to look it up on the phpunit documentation anyway.
Comment #2
clemens.tolboomIn #2100637-38: REST views: add special handling for collections test failed due to missing @group which seems to be required. So this is a 'bug' in the documentation.
So we need @group which is used to distinguish tests to run. See http://phpunit.de/manual/current/en/appendixes.annotations.html#appendix...
Comment #3
tim.plunkett@group is not required by PHPUnit, but it is currently required for our Simpletest code.
@covers/@coversDefaultClass are best practices, and are documented by PHPUnit.
Comment #4
clemens.tolboomI think all three need to be listed on https://www.drupal.org/node/1354 as our best practice seems to require these.
@group is defined @ phpunit too.
I prefer to have a complete list on mentioned page instead of letting people scatter around various resource. Ie @param and @return are listed too.
Comment #5
tim.plunkettFeel free.
Comment #6
clemens.tolboomThe page is locked and I'm not capable documenting the Drupal reason ... but some copy paste from phpunit probably will do.
Let's see what @jhodgdon think.
Comment #7
jhodgdonPlease do not add those to node/1354.
These are not API documentation tags. They are like annotation -- used by PHPUnit. We do not document annotation on node/1354, just documentation tags. PHPUnit and how to write tests are documented in this section: https://www.drupal.org/phpunit which should cover those "tags".
Comment #8
webchickOpening this back up for consideration, and escalating priority.
We got this report in Drupal Module Upgrader: #2499971: Tests are missing @coversDefaultClass annotations The reason for this was not readily apparent, since http://cgit.drupalcode.org/drupalmoduleupgrader/tree/tests/src/Unit/Plug... *does* have PHPDoc above the class. But it turns out that we're expecting some kind of special format of this PHPDoc in order to not trigger exceptions.
#2422019: Don't use reflection for parsing test annotations went in without a change record, so that's one problem. But people writing new D8 code will also need to know what their docblocks need to look like in order to not completely kill the test listing page.
Comment #9
jhodgdonOK. So.. We have a page -- no actually a whole section about writing PHPUnit tests specific to Drupal:
https://www.drupal.org/phpunit
There's not a single thing in that section about what the class doc header needs to be in order for Drupal's PHPUnit testing framework to correctly pick up tests. There definitely should be something added there, maybe on the file structure and namespaces page? https://www.drupal.org/node/2116043
For Simpletest, we also do not have a page on what the class files need to be for D8, what namespace, where to put them, etc. under https://www.drupal.org/simpletest -- and I think we should.
Can someone write these? I personally am not sure what all the requirements are... or if someone can make a quick post here with what the requirements are, and good examples to use, I can make a first pass at writing the pages and then someone can verify/fix errors.
Once that is done, I think the appropriate thing to do on node/1354 would be to:
a) Have a section on node/1354 about doc headers for PHPUnit and Simpletest (up amongst the pages like "Documenting functions" etc.), and rather than putting full docs there, link to the more comprehensive pages we just wrote explaining what to do.
b) In the tag reference section of node/1354, make sure any PHPUnit and/or Simpletest tags are referenced and the explanation section also points to those docs pages we've just written.
How's that for a plan?
Comment #10
webchickThat sounds like a lovely plan! I'd be happy to write the docs myself, but I have no idea what they should say. :( For now we are doing @coversDefaultClass because it seems to kill two birds with one stone, but I couldn't say what the "proper" template should be. https://api.drupal.org/api/drupal/core%21tests%21Drupal%21Tests%21UnitTe... shows a lot of inconsistency.
Comment #11
webchickOh, neat. In searching change records for @group, I found https://www.drupal.org/node/2301125 which looks like the ticket.
Comment #12
webchickOk here's a first step. Renamed that page from "PHPUnit file structure and namespace" to "PHPUnit file structure, namespace, and required metadata" and added in the content of the change record: https://www.drupal.org/node/2116043/revisions/view/8530219/8530253
I do not know how to write the page under /simpletest, but agree it should exist, and should likely mirror the PHPUnit page here, assuming that looks ok.
Comment #13
jhodgdonWeird, so we do not require a one-line description on PHPUnit test class doc blocks??!? Whatever.
OK, I wrote up what I at least think is accurate for Simpletest on https://www.drupal.org/node/2500059
If someone could review both that page and the revisions webchick made in #12, that would be good.
I'll meanwhile update node/1354
Comment #14
jhodgdonHm, I noticed the new page for PHPUnit doesn't say anything about @covers, so that needs to be added.
Anyway, I updated node/1354:
https://www.drupal.org/node/1354/revisions/view/8338507/8530331
Setting to Needs Work because the PHPUnit page needs @covers to be covered. Also both pages need reviews:
https://www.drupal.org/node/2500059 [completely new]
https://www.drupal.org/node/2116043/revisions/view/8530219/8530253 [revision]
Comment #15
webchick♪♬ Please don't shoot the messenger! :D ♫♭ I merely copy/pasted from the change record.
Good point on @covers. I actually have no idea what either it nor @coversDefaultClass does, so I'll leave that to someone else to document. (Basically, a one sentence description that links off to the PHPUnit docs.)
Comment #29
quietone commentedComment #30
quietone commentedComment #31
quietone commentedAt the recent coding standards meeting mstrelan reported that 'we need to use attributes' and not annotations. See the meeting issue for the full discussion, #3458726: Coding Standards Meeting Tuesday 2024-07-16 2100 UTC
The suggestion was that our docs should point to PHPUnit docs. I currently disagree with that because of the '@group'. There is no documentation to explain how that is used in Drupal. Sometimes it is upper case and sometime it is not. That should be made clear to developers.
Comment #32
quietone commentedConvert to new template.
Comment #33
borisson_I agree we can't just point to the phpunit docs, but I think we should link to them + explain our own additional rules about them if they exist.
Comment #34
mondrakeThis is now partially outdated, in the sense that annotations are deprecated in PHPUnit 10 in favor of PHP attributes, and there is no one to one mapping in all cases.
See https://github.com/sebastianbergmann/phpunit/issues/4502
Comment #35
quietone commentedComment #36
quietone commentedComment #37
quietone commented@groupand@coversDefaultClassare documented at Required PHPDoc metadata for test discoverabilityFrom #7. So, that means that page gets updated with the attribute information. And, that this should have stayed in the core queue.