Problem/Motivation

Discovered whilst RTBC-ing #3306715: Replace ckeditor with editor_test in editor_private_test.info.yml, the test modules that should be in core/modules/editor/tests/modules are a bit "wonky". Let's get them each in a submodule in a separate directory under core/modules/editor/tests/modules.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3307310

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

Spokje created an issue. See original summary.

spokje’s picture

spokje
Primary language Dutch
commented
Assigned: Unassigned » spokje
spokje’s picture

spokje
Primary language Dutch
commented
Title: Restructure core/modules/editor/tests/modules test modules » [PP-1] Restructure core/modules/editor/tests/modules test modules
Status: Active » Postponed

Postponed on #3306715: Replace ckeditor with editor_test in editor_private_test.info.yml

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
Title: [PP-1] Restructure core/modules/editor/tests/modules test modules » Restructure core/modules/editor/tests/modules test modules
Status: Postponed » Active

Un-postponed 😎

Spokje opened merge request !2720

spokje’s picture

spokje
Primary language Dutch
commented
Status: Active » Needs review

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

Change looks good.

Marking RTBC but may need a separate MR for 10.1.x.

The current MR does apply cleaning to 10.1.x though.

  • xjm committed 4b589664 on 10.1.x 
    Issue #3307310 by Spokje: Restructure core/modules/editor/tests/modules...

  • xjm committed 9255262c on 10.0.x 
    Issue #3307310 by Spokje: Restructure core/modules/editor/tests/modules...

  • xjm committed 81ef1475 on 9.5.x 
    Issue #3307310 by Spokje: Restructure core/modules/editor/tests/modules...
xjm’s picture

xjm
she/her
Primary language English
commented
Version: 10.1.x-dev » 9.5.x-dev
Status: Reviewed & tested by the community » Fixed

Wonky indeed.

I inspected the directory structure of core/modules/editor/tests both before and after the patch, and confirmed that all the fixtures are correctly moved to the subdirectory where one would expect to find them, and also that the only things left behind in the top-level src directory are the tests themselves.

Committed to 10.1.x, and cherry-picked to 10.0.x and 9.5.x as a test code cleanup. Thanks!

xjm closed merge request !2720

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Related issues: +#1833716: WYSIWYG: Introduce "Text editors" as part of filter format configuration

Hah, apparently we got this wrong ~10 years ago, when adding this test coverage in #1833716: WYSIWYG: Introduce "Text editors" as part of filter format configuration 😅 Sorry!

mondrake’s picture

mondrake
Location 🇮🇹
commented

Ah cool this is fixed already, I was hitting it at #3252386: Use PHP attributes instead of doctrine annotations.

spokje’s picture

spokje
Primary language Dutch
commented

Ah cool this is fixed already,

Drupal: Come for the code, stay for the cool fixes...

Status: Fixed » Closed (fixed)

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