Problem/Motivation

This issue is specifically to alter tests that are using the book module, but are not testing anything specific to the book module.

I could only find this one:
\Drupal\Tests\comment\Functional\CommentFieldsTest::testCommentInstallAfterContentModule uses the Book module, which is being deprecated.

Steps to reproduce

Proposed resolution

Use a test module instead of Book.

I think we can delete this entire test method

@catch in #12

Remaining tasks

Patch
Review
Commit

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3376369

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:

Comments

Spokje created an issue. See original summary.

spokje’s picture

Issue summary: View changes
spokje’s picture

Assigned: spokje » Unassigned

spokje’s picture

Status: Active » Needs review
smustgrave’s picture

Status: Needs review » Needs work

Did a light search but found

CommentBookTest
HandlerAllTest
TestViewsTest

Are also using book.

spokje’s picture

Status: Needs work » Needs review

I didn't do a full check, but I assume they are handled in #3376271: Move non-migration book-related tests to the Book module.

That one is for moving tests which actually test the book module to the book module.
This one is for handling tests that are "accidentally" using book, but are not using it for something that has specifically to do with book.

The IS (which I wrote isn't really helpful, sorry)

spokje’s picture

Issue summary: View changes
smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

If that's the only one the change looks good. We had something similar with tour module deprecation where there was 1 test that had to be tweaked. Was complex haha

spokje’s picture

If that's the only one the change looks good.

I think it is.

We had something similar with tour module deprecation where there was 1 test that had to be tweaked. Was complex haha

Tell me about, the ~15 lines of code in this MR almost took me 4 hrs...

quietone made their first commit to this issue’s fork.

catch’s picture

Status: Reviewed & tested by the community » Needs work

Sorry to be a pain after the four hours, but I think we can delete this entire test method instead, for the following reasons:

1. There is no such thing as a 'content type module' any more - the test dates from before bundle entities, when we still had hook_node_type() or whatever it was called in Drupal 7, so the situation it was designed to test doesn't really exist in the same way now.

2. CommentUninstallTest has kernel test coverage for the uninstall case (field_purge_batch() etc.), which is the only tricky thing the test actually covers now.

I got to this because I was wondering why we were modifying a test to delete a node type added by entity_test module, then saw the default config, then realised the mismatch between what the test thinks it's testing and what actually happens since 2014 or whenever things changed.

Spokje changed the visibility of the branch 3376369-remove-use-of to hidden.

spokje’s picture

Issue summary: View changes

Spokje changed the visibility of the branch 11.x to hidden.

spokje’s picture

Status: Needs work » Needs review

Sorry to be a pain after the four hours

No worries, it was quite educational.

but I think we can delete this entire test method instead,

Test deleted.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Simple review then.

  • catch committed 2c6bb82a on 11.x
    Issue #3376369 by Spokje, quietone, smustgrave, catch: Remove use of...

  • catch committed 1004c40d on 10.2.x
    Issue #3376369 by Spokje, quietone, smustgrave, catch: Remove use of...
catch’s picture

Version: 11.x-dev » 10.2.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 11.x and cherry-picked to 10.2.x, thanks!

Status: Fixed » Closed (fixed)

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