Problem/Motivation

As title.

Proposed resolution

For example:

-    $this->assertTrue(array_key_exists($this->nodes['public_no_language_public']->id(), $nids), 'Returned node ID is no language public node.');
+    $this->assertArrayHasKey($this->nodes['public_no_language_public']->id(), $nids);

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#14 rawdiff-11-14.txt10.66 KBjungle
#14 3131223-8.9.x-14.patch25.27 KBjungle
#11 interdiff-6-11.txt2.24 KBjungle
#11 3131223-11.patch25.28 KBjungle
#6 interdiff-3-6.txt18.4 KBjungle
#6 3131223-6.patch25.6 KBjungle
#3 3131223-3.patch27.4 KBjungle

Comments

mondrake created an issue. See original summary.

jungle’s picture

jungle
He/Him
Location Chongqing, China
commented
Assigned: Unassigned » jungle
jungle’s picture

jungle
He/Him
Location Chongqing, China
commented
Title: Replace assertions involving calls to array_key_exists() with assertArrayHasKey() » Replace assertions involving calls to array_key_exists() with assertArrayHasKey()/assertArrayNotHasKey()
Status: Active » Needs review
StatusFileSize
new 3131223-3.patch27.4 KB

Expanding the scope a little bit to have assertArrayNotHasKey() by changing the title

mondrake’s picture

mondrake
Location 🇮🇹
commented

I suggest to remove the $messages straight here... in this case PHPUnit default will be way better.

jungle’s picture

jungle
He/Him
Location Chongqing, China
commented

Thanks, @mondrake, on it.

jungle’s picture

jungle
He/Him
Location Chongqing, China
commented
StatusFileSize
new 3131223-6.patch25.6 KB
new interdiff-3-6.txt18.4 KB

Addressed #4

jungle’s picture

jungle
He/Him
Location Chongqing, China
commented
Assigned: jungle » Unassigned
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs review » Reviewed & tested by the community

Changes look good, no other instances of >assert.*array_key_exists.

mondrake’s picture

mondrake
Location 🇮🇹
commented 
+++ b/core/modules/node/tests/src/Kernel/NodeAccessLanguageAwareCombinationTest.php
@@ -278,9 +278,9 @@ public function testNodeAccessLanguageAwareCombination() {
+    $this->assertArrayHasKey($this->nodes['public_both_public']->id(), $nids, 'Returned node ID is both public node.');
+    $this->assertArrayHasKey($this->nodes['public_ca_private']->id(), $nids, 'Returned node ID is Hungarian public only node.');
+    $this->assertArrayHasKey($this->nodes['private_both_public']->id(), $nids, 'Returned node ID is both public non-language-aware private only node.');
 

@@ -292,9 +292,9 @@ public function testNodeAccessLanguageAwareCombination() {
+    $this->assertArrayHasKey($this->nodes['public_both_public']->id(), $nids, 'Returned node ID is both public node.');
+    $this->assertArrayHasKey($this->nodes['public_hu_private']->id(), $nids, 'Returned node ID is Catalan public only node.');
+    $this->assertArrayHasKey($this->nodes['private_both_public']->id(), $nids, 'Returned node ID is both public non-language-aware private only node.');

These still have the $message, leftover or is there a reasoning behind?

jungle’s picture

jungle
He/Him
Location Chongqing, China
commented

Thanks, @longwave and @mondrake. Yes, leftover. A new patch coming soon.

jungle’s picture

jungle
He/Him
Location Chongqing, China
commented
StatusFileSize
new 3131223-11.patch25.28 KB
new interdiff-6-11.txt2.24 KB

Addressed #9. Stay RTBC.

  • catch committed 54f84c2 on 9.1.x 
    Issue #3131223 by jungle, mondrake, longwave: Replace assertions...
catch’s picture

catch
he/him
Primary language English
commented
Version: 9.1.x-dev » 8.9.x-dev
Status: Reviewed & tested by the community » Needs work

Committed/pushed to 9.1.x and 8.0.x, needs a reroll for 8.9.x.

jungle’s picture

jungle
He/Him
Location Chongqing, China
commented
Status: Needs work » Needs review
StatusFileSize
new 3131223-8.9.x-14.patch25.27 KB
new rawdiff-11-14.txt10.66 KB
jungle’s picture

jungle
He/Him
Location Chongqing, China
commented
Status: Needs review » Needs work 
+++ b/Users/jungleran/3131223-11.patch
@@ -85,13 +85,13 @@ index 6886f22db6..2e750c8a65 100644
-     $this->assertEqual(count($nids), 3, 'db_select() returns 3 nodes when no langcode is specified.');
+     $this->assertEqual(count($nids), 3, '$connection->select() returns 3 nodes when no langcode is specified.');

A wrong comment, leftover message but looks like an unexpected change, reroll again. Sorry for the noises!

jungle’s picture

jungle
He/Him
Location Chongqing, China
commented

The reroll in #14 was correct, the raw-interdiff confused me. Sorry! Requeueing to run tests

  • catch committed 5b5c83a on 9.0.x 
    Issue #3131223 by jungle, mondrake, longwave: Replace assertions...
jungle’s picture

jungle
He/Him
Location Chongqing, China
commented

I am setting this back to RTBC as it's a reroll, and the testing passed as expected. Thanks, @longwave and @mondrake for reviewing, @catch thank you for committing!

jungle’s picture

jungle
He/Him
Location Chongqing, China
commented
Status: Needs work » Reviewed & tested by the community
catch’s picture

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

Committed 4afd6fb and pushed to 8.9.x. Thanks!

  • catch committed 4afd6fb on 8.9.x 
    Issue #3131223 by jungle, mondrake, longwave: Replace assertions...

Status: Fixed » Closed (fixed)

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