From a293b3aea802500c2c9d0c6e7ffc4c9935c7fbec Mon Sep 17 00:00:00 2001 From: "Eloy Lafuente (stronk7)" Date: Tue, 1 Sep 2020 00:07:51 +0200 Subject: [PATCH] MDL-67673 phpunit: Remove deprecated non-public attribute assertions 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 | 6 +++--- message/tests/api_test.php | 12 ++++++------ .../calculatedsimple/tests/questiontype_test.php | 6 +++--- .../type/description/tests/questiontype_test.php | 2 +- question/type/match/tests/questiontype_test.php | 6 +++--- .../type/multianswer/tests/questiontype_test.php | 12 ++++++------ .../type/multichoice/tests/questiontype_test.php | 8 ++++---- question/type/numerical/tests/questiontype_test.php | 6 +++--- .../type/shortanswer/tests/questiontype_test.php | 6 +++--- question/type/truefalse/tests/questiontype_test.php | 6 +++--- 10 files changed, 35 insertions(+), 35 deletions(-) diff --git a/badges/tests/badgeslib_test.php b/badges/tests/badgeslib_test.php index 4c5c05930a4..fc80b9b4ff2 100644 --- a/badges/tests/badgeslib_test.php +++ b/badges/tests/badgeslib_test.php @@ -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); } /** diff --git a/message/tests/api_test.php b/message/tests/api_test.php index 917ab4e3c0c..abcacd8f89d 100644 --- a/message/tests/api_test.php +++ b/message/tests/api_test.php @@ -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); diff --git a/question/type/calculatedsimple/tests/questiontype_test.php b/question/type/calculatedsimple/tests/questiontype_test.php index 35483c6732a..d18bd3f35f2 100644 --- a/question/type/calculatedsimple/tests/questiontype_test.php +++ b/question/type/calculatedsimple/tests/questiontype_test.php @@ -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); } } } diff --git a/question/type/description/tests/questiontype_test.php b/question/type/description/tests/questiontype_test.php index b2444800cee..7e0dd575be9 100644 --- a/question/type/description/tests/questiontype_test.php +++ b/question/type/description/tests/questiontype_test.php @@ -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); } } } diff --git a/question/type/match/tests/questiontype_test.php b/question/type/match/tests/questiontype_test.php index 0da8cacb219..dd9ee1c806e 100644 --- a/question/type/match/tests/questiontype_test.php +++ b/question/type/match/tests/questiontype_test.php @@ -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); } } } diff --git a/question/type/multianswer/tests/questiontype_test.php b/question/type/multianswer/tests/questiontype_test.php index eb8e6071d4b..defb2bda1da 100644 --- a/question/type/multianswer/tests/questiontype_test.php +++ b/question/type/multianswer/tests/questiontype_test.php @@ -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); } } } diff --git a/question/type/multichoice/tests/questiontype_test.php b/question/type/multichoice/tests/questiontype_test.php index c18ead0627c..7068cab481b 100644 --- a/question/type/multichoice/tests/questiontype_test.php +++ b/question/type/multichoice/tests/questiontype_test.php @@ -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); } } } diff --git a/question/type/numerical/tests/questiontype_test.php b/question/type/numerical/tests/questiontype_test.php index 9a39cde39f4..fef820cc586 100644 --- a/question/type/numerical/tests/questiontype_test.php +++ b/question/type/numerical/tests/questiontype_test.php @@ -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); } } } diff --git a/question/type/shortanswer/tests/questiontype_test.php b/question/type/shortanswer/tests/questiontype_test.php index 71ec1fe72fb..5a1ca8397c5 100644 --- a/question/type/shortanswer/tests/questiontype_test.php +++ b/question/type/shortanswer/tests/questiontype_test.php @@ -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); } } } diff --git a/question/type/truefalse/tests/questiontype_test.php b/question/type/truefalse/tests/questiontype_test.php index 1408a443cc6..6bae0bcdc27 100644 --- a/question/type/truefalse/tests/questiontype_test.php +++ b/question/type/truefalse/tests/questiontype_test.php @@ -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;