Skip to content

Commit

Permalink
MDL-67673 phpunit: Remove deprecated non-public attribute assertions
Browse files Browse the repository at this point in the history
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 sebastianbergmann/phpunit#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"
  • Loading branch information
stronk7 committed Oct 21, 2020
1 parent d95c378 commit a293b3a
Show file tree
Hide file tree
Showing 10 changed files with 35 additions and 35 deletions.
6 changes: 3 additions & 3 deletions badges/tests/badgeslib_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -199,15 +199,15 @@ public function test_badge_status() {
$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);
}

/**
Expand Down
12 changes: 6 additions & 6 deletions message/tests/api_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -5572,8 +5572,8 @@ public function test_send_message_to_conversation_individual_conversation() {
// 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(),
Expand Down Expand Up @@ -5614,8 +5614,8 @@ public function test_send_message_to_conversation_group_conversation() {
// 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);
Expand Down Expand Up @@ -5683,8 +5683,8 @@ public function test_send_message_to_conversation_linked_group_conversation() {
// 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);
Expand Down
6 changes: 3 additions & 3 deletions question/type/calculatedsimple/tests/questiontype_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -90,21 +90,21 @@ public function test_question_saving_sumwithvariants() {

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);
}
}

foreach ($questiondata->options->answers as $answer) {
$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);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion question/type/description/tests/questiontype_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public function test_question_saving() {

foreach ($questiondata as $property => $value) {
if (!in_array($property, array('id', 'version', 'timemodified', 'timecreated'))) {
$this->assertAttributeEquals($value, $property, $actualquestiondata);
$this->assertEquals($value, $actualquestiondata->$property);
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions question/type/match/tests/questiontype_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -185,13 +185,13 @@ public function test_question_saving_foursubq() {

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);
}
}

Expand All @@ -202,7 +202,7 @@ public function test_question_saving_foursubq() {
$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);
}
}
}
Expand Down
12 changes: 6 additions & 6 deletions question/type/multianswer/tests/questiontype_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -251,21 +251,21 @@ public function test_question_saving_twosubq() {

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);
}
}

foreach ($questiondata->hints as $hint) {
$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);
}
}
}
Expand All @@ -279,20 +279,20 @@ public function test_question_saving_twosubq() {
$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) {
$actualanswer = array_shift($actualsubq->options->answers);
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);
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions question/type/multichoice/tests/questiontype_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -134,21 +134,21 @@ public function test_question_saving_two_of_four($which) {

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);
}
}

foreach ($questiondata->hints as $hint) {
$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);
}
}
}
Expand All @@ -158,7 +158,7 @@ public function test_question_saving_two_of_four($which) {
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);
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions question/type/numerical/tests/questiontype_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -149,13 +149,13 @@ public function test_question_saving_pi() {

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);
}
}

Expand All @@ -164,7 +164,7 @@ public function test_question_saving_pi() {
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);
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions question/type/shortanswer/tests/questiontype_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,13 @@ public function test_question_saving_frogtoad() {

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);
}
}

Expand All @@ -136,7 +136,7 @@ public function test_question_saving_frogtoad() {
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);
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions question/type/truefalse/tests/questiontype_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -161,13 +161,13 @@ public function test_question_saving_true() {

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);
}
}

Expand All @@ -177,7 +177,7 @@ public function test_question_saving_true() {
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;
Expand Down

0 comments on commit a293b3a

Please sign in to comment.