- Merge request !5917Resolve #2791061 "Blockcontentrevisionstest should use" → (Open) created by smustgrave
- Status changed to Needs review
about 1 year ago 3:31pm 21 December 2023 - Status changed to Needs work
about 1 year ago 9:12pm 2 January 2024 - 🇦🇺Australia acbramley
It's not clear to me from the comments or IS why exactly we need to login as the admin user?
- 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
+1 for logging in as a user with the minimum set of permissions rather than the admin user. Using the admin user masks subtle bugs.
- Status changed to Needs review
about 1 year ago 3:36pm 3 January 2024 - 🇺🇸United States smustgrave
Slightly tweaked based on @larowlan comment in #29
Addressed feedback.
- Status changed to Needs work
about 1 year ago 10:01pm 7 January 2024 - 🇦🇺Australia acbramley
Updated the IS to be more accurate. I'm honestly questioning whether we need this test at all. It's testing the most arbitrary stuff and was first added in a very similar state 10 years ago https://git.drupalcode.org/project/drupal/-/commit/b2320bc9d58ee9c5df269...
What the test does (currently):
- Checks the log message set by setRevisionLogMessage in setUp is the same when loading the block revision - Seems redundant.
- Checks the revision user, revision id and revision create time are the correct data types, except for the first revision - Very redundant and also exposes a bug that I've reported elsewhere (will find the link soon)
- Checks the latest revision is the default revision - Also redundant.
- Checks unpublished/non default revision's body text doesn't appear on the frontpage - Possibly useful
- Checks the block edit form is a 200 and doesn't contain the latest revision's body text - Redundant and potentially a bug since the edit form SHOULD contain the latest revision's content
- Checks the latest revision id is greater than the default revision id - very reundant
- Checks the frontpage contains the default revision's body text - possibly useful
When I say redundant I mean it's probably tested elsewhere, or completely useless.
Setting to NW because honestly I reckon we ditch the test entirely after confirming we're covered elsewhere.
- 🇺🇸United States smustgrave
So just checking item 1 this appears to be the only test using setRevisionLogMessage and testing the message on the version-history page.
So not 100% sure it is covered elsewhere.
- 🇦🇺Australia acbramley
this appears to be the only test using setRevisionLogMessage and testing the message on the version-history page.
But it doesn't check the version history page? It checks getRevisionLogMessage returns what was set in setUp
We do have other testing for the version history page though! https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/block...
