Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
#2073811-154: Add a url generator twig extension got me thinking that we're missing test coverage and sure enough we have no test coverage of visiting node/add
with no node types configured.
Proposed resolution
Add missing test coverage toDrupal\node\Tests\NodeCreationTest
- note similar to testNodeCreation
you will need to delete the content types created during setUp at the beginning of the test.
Remaining tasks
Task | Novice task? | Contributor instructions | Complete? |
---|---|---|---|
Create a patch | Y | Instructions | Y |
Update the issue summary | Y | Instructions | Y |
Add automated tests | Y | Instructions | Y |
User interface changes
API changes
Comment | File | Size | Author |
---|---|---|---|
#23 | interdiff-2309673-21-23.txt | 444 bytes | pbull |
#23 | d7-test-no-node-type-exist-2309673-23.patch | 1.19 KB | pbull |
#21 | d7-test-no-node-type-exist-2309673-21.patch | 1.14 KB | pbull |
#15 | interdiff-2309673-11-15.txt | 534 bytes | mitsuroseba |
#15 | test-no-node-type-exist-2309673-15.patch | 1.11 KB | mitsuroseba |
Comments
Comment #1
alexpottComment #2
mitsuroseba CreditAttribution: mitsuroseba commentedComment #3
anavarreTrying to understand what's the scope for this issue: should the test live in a new function or would adding it in the
testNodeCreation()
function be acceptable?Also, I did try to add the below test (temporarily) in the
testNodeCreation()
function but then it throws a Simpletest failure as a 403 is returned while a 200 is expected:I've noticed that the only way to fix it is to add the 'administer content types' permission to the user object.
P.S.: I did try that with and without Cottser's patch in #2073811: Add a url generator twig extension
Comment #4
alexpottA full test of the node/add functionality would include testing for users with and without
'administer content types'
. For users with the'administer content types'
permission we should be using WebTest's click link functionality to test that the expected link is there.Comment #5
alexpottCottser's patch in #2073811: Add a url generator twig extension is fine and works it's @dawehner's patch that exposed the missing test coverage.
Comment #6
mitsuroseba CreditAttribution: mitsuroseba commentedComment #7
alexpottNice work - a few minor things to keep the test as simple and reliable as possible.
Lets call this
testNodeAddWithoutContentTypes
since you're not creating a node.I would assert that the current user can get to node/add before deleting the content types - in it's current state this seems a bit brittle.
You only really need to do this is you are expecting the url to be different as is the case in testNodeCreation
We don't need to actually go to the node type add page - this is tested elsewhere.
Comment #8
mitsuroseba CreditAttribution: mitsuroseba commentedThank you for review. Here a new patch.
Comment #9
mitsuroseba CreditAttribution: mitsuroseba commentedComment #10
alexpottNearly perfect! Just a couple of very very minor nitpicks.
Actually there is no need to assert the 200 here if you are going to assert the link. And the assertLinkByHref's second argument is not needed. The default is 0.
Comment #11
mitsuroseba CreditAttribution: mitsuroseba commentedComment #12
alexpottGreat! Thank you.
Comment #13
dawehnerCan we have a assertNoLinkByHref in case we do have node types?
Comment #14
alexpottGood point @dawehner
Comment #15
mitsuroseba CreditAttribution: mitsuroseba commentedComment #16
alexpottThanks!
Comment #17
dawehnerperfect
Comment #19
catchCommitted/pushed to 8.0.x, thanks!
Could probably be backported.
Comment #21
pbull CreditAttribution: pbull commentedHere's a D7 backport that adds this test to NodeCreationTestCase.
Comment #23
pbull CreditAttribution: pbull commentedTrying again... previous D7 patch passes locally but this adds node and menu rebuilds.
Comment #25
m1r1k CreditAttribution: m1r1k commentedComment #27
mitsuroseba CreditAttribution: mitsuroseba commentedRetest