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
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:
- 3376369-delete-test
changes, plain diff MR !6124
- 11.x
compare
- 3376369-remove-use-of
changes, plain diff MR !5988
Comments
Comment #2
spokjeComment #3
spokjeComment #5
spokjeComment #6
smustgrave commentedDid a light search but found
CommentBookTest
HandlerAllTest
TestViewsTest
Are also using book.
Comment #7
spokjeI 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)
Comment #8
spokjeComment #9
smustgrave commentedIf 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
Comment #10
spokjeI think it is.
Tell me about, the ~15 lines of code in this MR almost took me 4 hrs...
Comment #12
catchSorry 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.
Comment #15
spokjeComment #18
spokjeNo worries, it was quite educational.
Test deleted.
Comment #19
smustgrave commentedSimple review then.
Comment #22
catchCommitted/pushed to 11.x and cherry-picked to 10.2.x, thanks!