MDL-67673 phpunit: Remove deprecated non-public attribute assertions
authorEloy Lafuente (stronk7) <stronk7@moodle.org>
Mon, 31 Aug 2020 22:07:51 +0000 (00:07 +0200)
committerEloy Lafuente (stronk7) <stronk7@moodle.org>
Wed, 21 Oct 2020 10:46:05 +0000 (12:46 +0200)
With PHPUnit 8 a good number of assertions, all them related with
operations on non-public attributes have been deprecated. And will
be removed with PHPUnit 9.

The main point is that unit tests shouldn't be testing non-public
APIs (good practice) and those assertions were an error originally.

See https://github.com/sebastianbergmann/phpunit/issues/3338 for
the complete list and other details.

When possible (the attributes being checked are public), the change
is simple, just switching to normal assertions.

When the attributes are not public we need to find a workaround
to be able to test the same using public APIs, or use Reflection,
or remove the tests.

For the records, this is the regexp used to find all the cases:

ag '>(assertAttribute|attribute\(|readAttributte|getStaticAttribute| \
    getObjectAttribute)' -G "test.php"

badges/tests/badgeslib_test.php
message/tests/api_test.php
question/type/calculatedsimple/tests/questiontype_test.php
question/type/description/tests/questiontype_test.php
question/type/match/tests/questiontype_test.php
question/type/multianswer/tests/questiontype_test.php
question/type/multichoice/tests/questiontype_test.php
question/type/numerical/tests/questiontype_test.php
question/type/shortanswer/tests/questiontype_test.php
question/type/truefalse/tests/questiontype_test.php

index 4c5c059..fc80b9b 100644 (file)
@@ -199,15 +199,15 @@ class core_badges_badgeslib_testcase extends advanced_testcase {
         $badge = new badge($this->badgeid);
         $old_status = $badge->status;
         $badge->set_status(BADGE_STATUS_ACTIVE);
-        $this->assertAttributeNotEquals($old_status, 'status', $badge);
-        $this->assertAttributeEquals(BADGE_STATUS_ACTIVE, 'status', $badge);
+        $this->assertNotEquals($old_status, $badge->status);
+        $this->assertEquals(BADGE_STATUS_ACTIVE, $badge->status);
     }
 
     public function test_delete_badge() {
         $badge = new badge($this->badgeid);
         $badge->delete();
         // We don't actually delete badges. We archive them.
-        $this->assertAttributeEquals(BADGE_STATUS_ARCHIVED, 'status', $badge);
+        $this->assertEquals(BADGE_STATUS_ARCHIVED, $badge->status);
     }
 
     /**
index 917ab4e..abcacd8 100644 (file)
@@ -5572,8 +5572,8 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
         // Verify the message returned.
         $this->assertInstanceOf(\stdClass::class, $message1);
         $this->assertObjectHasAttribute('id', $message1);
-        $this->assertAttributeEquals($user1->id, 'useridfrom', $message1);
-        $this->assertAttributeEquals('this is a message', 'text', $message1);
+        $this->assertEquals($user1->id, $message1->useridfrom);
+        $this->assertEquals('this is a message', $message1->text);
         $this->assertObjectHasAttribute('timecreated', $message1);
 
         // Verify events. Note: the event is a message read event because of an if (PHPUNIT) conditional within message_send(),
@@ -5614,8 +5614,8 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
         // Verify the message returned.
         $this->assertInstanceOf(\stdClass::class, $message1);
         $this->assertObjectHasAttribute('id', $message1);
-        $this->assertAttributeEquals($user1->id, 'useridfrom', $message1);
-        $this->assertAttributeEquals('message to the group', 'text', $message1);
+        $this->assertEquals($user1->id, $message1->useridfrom);
+        $this->assertEquals('message to the group', $message1->text);
         $this->assertObjectHasAttribute('timecreated', $message1);
         // Test customdata.
         $customdata = json_decode($messages[0]->customdata);
@@ -5683,8 +5683,8 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
         // Verify the message returned.
         $this->assertInstanceOf(\stdClass::class, $message1);
         $this->assertObjectHasAttribute('id', $message1);
-        $this->assertAttributeEquals($user1->id, 'useridfrom', $message1);
-        $this->assertAttributeEquals('message to the group', 'text', $message1);
+        $this->assertEquals($user1->id, $message1->useridfrom);
+        $this->assertEquals('message to the group', $message1->text);
         $this->assertObjectHasAttribute('timecreated', $message1);
         // Test customdata.
         $customdata = json_decode($messages[0]->customdata);
index 35483c6..d18bd3f 100644 (file)
@@ -90,13 +90,13 @@ class qtype_calculatedsimple_test extends advanced_testcase {
 
         foreach ($questiondata as $property => $value) {
             if (!in_array($property, array('id', 'version', 'timemodified', 'timecreated', 'options'))) {
-                $this->assertAttributeEquals($value, $property, $actualquestiondata);
+                $this->assertEquals($value, $actualquestiondata->$property);
             }
         }
 
         foreach ($questiondata->options as $optionname => $value) {
             if ($optionname != 'answers') {
-                $this->assertAttributeEquals($value, $optionname, $actualquestiondata->options);
+                $this->assertEquals($value, $actualquestiondata->options->$optionname);
             }
         }
 
@@ -104,7 +104,7 @@ class qtype_calculatedsimple_test extends advanced_testcase {
             $actualanswer = array_shift($actualquestiondata->options->answers);
             foreach ($answer as $ansproperty => $ansvalue) {
                 if (!in_array($ansproperty, array('id', 'question', 'answerformat'))) {
-                    $this->assertAttributeEquals($ansvalue, $ansproperty, $actualanswer);
+                    $this->assertEquals($ansvalue, $actualanswer->$ansproperty);
                 }
             }
         }
index b244480..7e0dd57 100644 (file)
@@ -95,7 +95,7 @@ class qtype_description_test extends advanced_testcase {
 
         foreach ($questiondata as $property => $value) {
             if (!in_array($property, array('id', 'version', 'timemodified', 'timecreated'))) {
-                $this->assertAttributeEquals($value, $property, $actualquestiondata);
+                $this->assertEquals($value, $actualquestiondata->$property);
             }
         }
     }
index 0da8cac..dd9ee1c 100644 (file)
@@ -185,13 +185,13 @@ class qtype_match_test extends advanced_testcase {
 
         foreach ($questiondata as $property => $value) {
             if (!in_array($property, array('id', 'version', 'timemodified', 'timecreated', 'options', 'stamp'))) {
-                $this->assertAttributeEquals($value, $property, $actualquestiondata);
+                $this->assertEquals($value, $actualquestiondata->$property);
             }
         }
 
         foreach ($questiondata->options as $optionname => $value) {
             if ($optionname != 'subquestions') {
-                $this->assertAttributeEquals($value, $optionname, $actualquestiondata->options);
+                $this->assertEquals($value, $actualquestiondata->options->$optionname);
             }
         }
 
@@ -202,7 +202,7 @@ class qtype_match_test extends advanced_testcase {
             $actualsubq = array_shift($actualquestiondata->options->subquestions);
             foreach ($subq as $subqproperty => $subqvalue) {
                 if (!in_array($subqproperty, $subqpropstoignore)) {
-                    $this->assertAttributeEquals($subqvalue, $subqproperty, $actualsubq);
+                    $this->assertEquals($subqvalue, $actualsubq->$subqproperty);
                 }
             }
         }
index eb8e607..defb2bd 100644 (file)
@@ -251,13 +251,13 @@ class qtype_multianswer_test extends advanced_testcase {
 
         foreach ($questiondata as $property => $value) {
             if (!in_array($property, array('id', 'version', 'timemodified', 'timecreated', 'options', 'hints', 'stamp'))) {
-                $this->assertAttributeEquals($value, $property, $actualquestiondata);
+                $this->assertEquals($value, $actualquestiondata->$property);
             }
         }
 
         foreach ($questiondata->options as $optionname => $value) {
             if ($optionname != 'questions') {
-                $this->assertAttributeEquals($value, $optionname, $actualquestiondata->options);
+                $this->assertEquals($value, $actualquestiondata->options->$optionname);
             }
         }
 
@@ -265,7 +265,7 @@ class qtype_multianswer_test extends advanced_testcase {
             $actualhint = array_shift($actualquestiondata->hints);
             foreach ($hint as $property => $value) {
                 if (!in_array($property, array('id', 'questionid', 'options'))) {
-                    $this->assertAttributeEquals($value, $property, $actualhint);
+                    $this->assertEquals($value, $actualhint->$property);
                 }
             }
         }
@@ -279,12 +279,12 @@ class qtype_multianswer_test extends advanced_testcase {
             $actualsubq = $actualquestiondata->options->questions[$subqno];
             foreach ($subq as $subqproperty => $subqvalue) {
                 if (!in_array($subqproperty, $subqpropstoignore)) {
-                    $this->assertAttributeEquals($subqvalue, $subqproperty, $actualsubq);
+                    $this->assertEquals($subqvalue, $actualsubq->$subqproperty);
                 }
             }
             foreach ($subq->options as $optionname => $value) {
                 if (!in_array($optionname, array('answers'))) {
-                    $this->assertAttributeEquals($value, $optionname, $actualsubq->options);
+                    $this->assertEquals($value, $actualsubq->options->$optionname);
                 }
             }
             foreach ($subq->options->answers as $answer) {
@@ -292,7 +292,7 @@ class qtype_multianswer_test extends advanced_testcase {
                 foreach ($answer as $ansproperty => $ansvalue) {
                     // These questions do not use 'answerformat', will ignore it.
                     if (!in_array($ansproperty, array('id', 'question', 'answerformat'))) {
-                        $this->assertAttributeEquals($ansvalue, $ansproperty, $actualanswer);
+                        $this->assertEquals($ansvalue, $actualanswer->$ansproperty);
                     }
                 }
             }
index c18ead0..7068cab 100644 (file)
@@ -134,13 +134,13 @@ class qtype_multichoice_test extends advanced_testcase {
 
         foreach ($questiondata as $property => $value) {
             if (!in_array($property, array('id', 'version', 'timemodified', 'timecreated', 'options', 'hints', 'stamp'))) {
-                $this->assertAttributeEquals($value, $property, $actualquestiondata);
+                $this->assertEquals($value, $actualquestiondata->$property);
             }
         }
 
         foreach ($questiondata->options as $optionname => $value) {
             if ($optionname != 'answers') {
-                $this->assertAttributeEquals($value, $optionname, $actualquestiondata->options);
+                $this->assertEquals($value, $actualquestiondata->options->$optionname);
             }
         }
 
@@ -148,7 +148,7 @@ class qtype_multichoice_test extends advanced_testcase {
             $actualhint = array_shift($actualquestiondata->hints);
             foreach ($hint as $property => $value) {
                 if (!in_array($property, array('id', 'questionid', 'options'))) {
-                    $this->assertAttributeEquals($value, $property, $actualhint);
+                    $this->assertEquals($value, $actualhint->$property);
                 }
             }
         }
@@ -158,7 +158,7 @@ class qtype_multichoice_test extends advanced_testcase {
             foreach ($answer as $ansproperty => $ansvalue) {
                 // This question does not use 'answerformat', will ignore it.
                 if (!in_array($ansproperty, array('id', 'question', 'answerformat'))) {
-                    $this->assertAttributeEquals($ansvalue, $ansproperty, $actualanswer);
+                    $this->assertEquals($ansvalue, $actualanswer->$ansproperty);
                 }
             }
         }
index 9a39cde..fef820c 100644 (file)
@@ -149,13 +149,13 @@ class qtype_numerical_test extends advanced_testcase {
 
         foreach ($questiondata as $property => $value) {
             if (!in_array($property, array('options'))) {
-                $this->assertAttributeEquals($value, $property, $actualquestiondata);
+                $this->assertEquals($value, $actualquestiondata->$property);
             }
         }
 
         foreach ($questiondata->options as $optionname => $value) {
             if (!in_array($optionname, array('answers'))) {
-                $this->assertAttributeEquals($value, $optionname, $actualquestiondata->options);
+                $this->assertEquals($value, $actualquestiondata->options->$optionname);
             }
         }
 
@@ -164,7 +164,7 @@ class qtype_numerical_test extends advanced_testcase {
             foreach ($answer as $ansproperty => $ansvalue) {
                 // This question does not use 'answerformat', will ignore it.
                 if (!in_array($ansproperty, array('id', 'question', 'answerformat'))) {
-                    $this->assertAttributeEquals($ansvalue, $ansproperty, $actualanswer);
+                    $this->assertEquals($ansvalue, $actualanswer->$ansproperty);
                 }
             }
         }
index 71ec1fe..5a1ca83 100644 (file)
@@ -121,13 +121,13 @@ class qtype_shortanswer_test extends advanced_testcase {
 
         foreach ($questiondata as $property => $value) {
             if (!in_array($property, array('id', 'version', 'timemodified', 'timecreated', 'options'))) {
-                $this->assertAttributeEquals($value, $property, $actualquestiondata);
+                $this->assertEquals($value, $actualquestiondata->$property);
             }
         }
 
         foreach ($questiondata->options as $optionname => $value) {
             if ($optionname != 'answers') {
-                $this->assertAttributeEquals($value, $optionname, $actualquestiondata->options);
+                $this->assertEquals($value, $actualquestiondata->options->$optionname);
             }
         }
 
@@ -136,7 +136,7 @@ class qtype_shortanswer_test extends advanced_testcase {
             foreach ($answer as $ansproperty => $ansvalue) {
                 // This question does not use 'answerformat', will ignore it.
                 if (!in_array($ansproperty, array('id', 'question', 'answerformat'))) {
-                    $this->assertAttributeEquals($ansvalue, $ansproperty, $actualanswer);
+                    $this->assertEquals($ansvalue, $actualanswer->$ansproperty);
                 }
             }
         }
index 1408a44..6bae0bc 100644 (file)
@@ -161,13 +161,13 @@ class qtype_truefalse_test extends advanced_testcase {
 
         foreach ($questiondata as $property => $value) {
             if (!in_array($property, array('options'))) {
-                $this->assertAttributeEquals($value, $property, $actualquestiondata);
+                $this->assertEquals($value, $actualquestiondata->$property);
             }
         }
 
         foreach ($questiondata->options as $optionname => $value) {
             if (!in_array($optionname, array('trueanswer', 'falseanswer', 'answers'))) {
-                $this->assertAttributeEquals($value, $optionname, $actualquestiondata->options);
+                $this->assertEquals($value, $actualquestiondata->options->$optionname);
             }
         }
 
@@ -177,7 +177,7 @@ class qtype_truefalse_test extends advanced_testcase {
             foreach ($answer as $ansproperty => $ansvalue) {
                 // This question does not use 'answerformat', will ignore it.
                 if (!in_array($ansproperty, array('id', 'question', 'answerformat'))) {
-                    $this->assertAttributeEquals($ansvalue, $ansproperty, $actualanswer);
+                    $this->assertEquals($ansvalue, $actualanswer->$ansproperty);
                 }
             }
             $answerindexes[$answer->answer] = $ansindex;