With the items in #3591027: [META] Remove use case specific elements from Standard and Node module. Always install with Standard. Keep Minimal for CLI use only. Standard is becoming more like a starting point instead of covering a real life use case. Especially following #3572350: Disable the default /node listing view, replace Olivero hardcoded welcome page from /node with welcome message the default front page will not be a node listing. While that is moved to a dedicated disabled view and its tests are moved there, the StandardPerformanceTest remains but is now not very useful to test real life scenarios. It is a much lower level baseline now.
@berdir suggested I open an issue to move the StandardPerformanceTest to more useful tests, eg. to Umami performance testing.
| Comment | File | Size | Author |
|---|
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes, plain diff MR !16936
Comments
Comment #2
gábor hojtsyPostponing on #3572350: Disable the default /node listing view, replace Olivero hardcoded welcome page from /node with welcome message.
Comment #3
gábor hojtsyThe base issue landed. Also @berdir pointed out we also have stuff to discuss in StandardJavascriptTest, so adding this to scope to make it easier since both are standard profile performance.
Comment #4
catchLooking for this I rediscovered #3426303: Move login performance testing to Umami which was suggesting moving the login test to Umami.
Comment #5
catchPut an MR up on #3426303: Move login performance testing to Umami to move just the login testing - we should definitely keep that, and it should definitely move to the Umami test IMO.
Once that's moved, then I think we can review the rest of the test + StandardJavascriptTest here.
I think the main thing that's useful with the standard performance tests is keeping track of absolute baseline queries/cache operations/css/js that core requires to serve a page - this is why the tests already use stark instead of Olivero. But also we have NodePerformanceTest which does some of that now too.
Comment #6
berdirWith #3590882: Standard profile should not include Olivero anymore, should provide guidance on how to get started, StandardPerformanceTest gets even less meaningful I think.
For example, testLogin() now asserts an "access_denied" watchdog insert, which means the user that's being created here doesn't have access to visit the page afterwards and is just testing the access denied result. But that we're moving to umami, so we'll see what's left after that issue I guess.
Comment #7
gábor hojtsyYeah its likely that following #3590882: Standard profile should not include Olivero anymore, should provide guidance on how to get started the StandardPerformanceTest is probably very close to useless and should move parts elsewhere.
Comment #8
catch#3426303: Move login performance testing to Umami is RTBC which removes user login from this test.
We have
NodePerformanceTestso I removed the node creation and assertions from the test.StandardJavaScriptTest predates performance testing as such, and I think we have everything in there covered between other performance tests and big pipe tests etc.
That leaves the default front page + admin logged in test which I think are worth leaving in - unless we moved the navigation test back to navigation module again.
MR is stacked on top of #3426303: Move login performance testing to Umami.
(edited to fix issue link)
Comment #10
berdirneeds a rebase as the blocker is in, also added a comment to the MR.
Comment #11
catchRebased and addressed the comment.
Comment #12
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #13
catchAddressed @berdir's last comment. I do think the remaining test method is useful, but we could potentially move that (back) to navigation at this point.
Comment #14
berdirNeeds an update for phpcs.
Comment #15
catchComment #16
berdirMakes sense to me. This is now testing only what's actually in standard, which is the admin experience and doesn't add any additional node types or changes permissions, We have NodePerfomanceTest for the minimal node/entity performance tests.
Comment #17
longwaveAn existing comment needs updating.
Comment #18
catchUpdated the comment - just a one word change so moving back to RTBC.
Comment #19
berdirThis needs a rebase because #2025089: Deprecate user_role_grant_permissions(), user_role_revoke_permissions() and user_role_change_permissions() changed a line that's being removed.
Comment #20
catchRebased.
Comment #22
longwaveCommitted and pushed cdefa18d849 to main. Thanks!