Problem/Motivation

I had another go at converting some core tests along the same process as #3583587: Convert some functional tests in system module to kernel tests, but mostly manually. Claude helped a little but I take responsibility for all the changes. I found that my process for gathering test timings last time was exaggerated due to built-in php server and sqlite, it's much faster with a php-fpm and mysql setup, so I've used that this time. This includes the conversion of one #slow test, which is now absolutely not slow.

| Test                       | Before    | After   | Saved    |
|----------------------------|-----------|---------|----------|
| ComponentRenderTest        | 2.806s    | 1.315s  | 1.491s   |
| EntityConcurrentRenderTest | 6.683s    | 1.582s  | 5.101s   |
| JqueryUiLibraryAssetsTest  | 15.070s   | 5.291s  | 9.779s   |
| ManyAssetsLoadOrderTest    | 2.679s    | 1.095s  | 1.584s   |
| PathEncodedTest            | 5.252s    | 1.673s  | 3.579s   |
| RevisionRouteProviderTest  | 5.765s    | 1.792s  | 3.973s   |
| RevisionVersionHistoryTest | 1m13.799s | 12.914s | 1m0.885s |
| RevisionViewTest           | 13.217s   | 3.168s  | 10.049s  |

Total before: 1m45.271s (105.271s)  
Total after: 28.830s  
Total time saved: 1m16.441s (76.441s)  
Reduction: 72.6%

Note there was already a Kernel test for ComponentRenderTest and RevisionRouteProviderTest so I've moved the test cases inside the existing classes.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#4 3588363-nr-bot_op4lts_5.txt98 bytesneeds-review-queue-bot

Issue fork drupal-3588363

Command icon Show commands

Start 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:

About issue forks

Comments

mstrelan created an issue. See original summary.

mstrelan opened merge request !15659

mstrelan’s picture

mstrelan commented
Status: Active » Needs review

Left some review comments. It should be needs work, but seeking other reviews first.

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3588363-nr-bot_op4lts_5.txt98 bytes

The Needs Review Queue Bot tested this issue. The merge request has merge conflicts and cannot be merged. 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.

mstrelan’s picture

mstrelan commented
Status: Needs work » Needs review
smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Left some comments on the MR.

mstrelan’s picture

mstrelan commented
Status: Needs work » Needs review

Fixed one and responded to the rest. Need a decision on ::resetPagerState to move forward.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Thanks for humoring me @mstrelan. Changes LGTM 

Error         6.639s testInstallProfileConfigOverwrite                                               
Failure              *** Process execution output ***                                                
    PHPUnit 12.5.14 by Sebastian Bergmann and contributors.
    
    Runtime:       PHP 8.5.8
    Configuration: /builds/core/phpunit.xml.dist
    
    E                                                                   1 / 1 (100%)
    
    Time: 00:06.957, Memory: 14.00 MB
    
    Config Install Profile Override (Drupal\Tests\config\Functional\ConfigInstallProfileOverride)
     ✘ Install profile config overwrite
       ┐
       ├ Error: Call to a member function label() on null
       │
       │ /builds/core/modules/config/tests/src/Functional/ConfigInstallProfileOverrideTest.php:141

Not related.

  • catch committed d512b5e6 on main 
    task: #3588363 Convert some tests in Drupal\FunctionalTests namespace to...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to main, thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

catch closed merge request !15659

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.