Remove deprecated assertion usage

[codisart]

Q	A
Documentation	no
Bugfix	no
BC Break	no
New Feature	no
RFC	no
QA	yes

Description

Remove some deprecated phpunit assertions that test non public attribute

[froschdesign]

I think with this type of test only duplicates are created.
For instance: the message text of Between::VALUE_NOT_NUMERIC is already tested in
BetweenTest ::testStringValidatedAgainstNumericMinAndMaxIsInvalidAndReturnsAFailureMessage.

laminas-validator/test/BetweenTest.php

Lines 292 to 301 in 7d81900

	public function testStringValidatedAgainstNumericMinAndMaxIsInvalidAndReturnsAFailureMessage()
	{
	$validator = new Between(['min' => 1, 'max' => 10]);
	$this->assertFalse($validator->isValid('a'));
	$messages = $validator->getMessages();
	$this->assertContains(
	'The min (\'1\') and max (\'10\') values are numeric, but the input is not',
	$messages
	);
	}

If the messages are to be compared then maybe test only for the keys by using the class constants.

[codisart]

Hello @froschdesign

The tests that I have replaced focused only on the error messages configuration. To be able to remove the most of the usage of assertAttributeEquals, I tought it best to keep for now the same intention of these olds tests. The only change that I have made is to use the real strings instead of the keys in order to not couple the tests with the classes that they test.

To be honest, I don't see a lot of value with these tests. I would prefer to create the case where these message are used. I propose to amend my PR to remove the test methods for the test classes where all the messages are already tested. What do you think ?

[froschdesign]

@codisart
The content of the messages should not be tested in all testEqualsMessageTemplates tests. But there is another option if you do not want to throw out the tests:

public function testEqualsMessageTemplates()
{
    $validator = new Between(['min' => 1, 'max' => 10]);
    $this->assertSame(
        [
            Between::NOT_BETWEEN,
            Between::NOT_BETWEEN_STRICT,
            Between::VALUE_NOT_NUMERIC,
            Between::VALUE_NOT_STRING,
        ],
        array_keys($validator->getMessageTemplates())
    );
}

Please feel free add your own proposal to find the best solution.

(Other improvements of the tests should be done in a separate pull requests because we should stay on the topic "Prepare update to phpunit 9".)

[codisart]

@froschdesign
Don't you think that to use directly the class constants add coupling between the test and the tested class ?

I am more and more inclined to just throw these tests.

[codisart]

I used the class constants for the tests as you suggested.

[codisart]

Hello @froschdesign, are my last changes ok with your recommendations ?

[michalbundyra]

any reason why this test has been removed?

[codisart]

I think it is an error. I will put it back.

[codisart]

Hello @michalbundyra,

Do I need to change things for this PR ?

[boesing]

Oh snap... I wasn't aware of this PR.
By adding support for PHP 8.0 in #75, PHPUnit 9.X was also necessary there.
Sadly, the author of that PR did not see this PR as he could used it as a starting point.

@codisart Could you compare your changes with those from #75? If you have additional changes for better support of phpunit 9, I'd suggest to rebase against that branch as it will likely be merged in favor of your PR due to the adoption of PHP 8.0.

[codisart]

Hello @boesing

There is a lot of differences between my fixes of the removal of "assertAttributeEquals" with #75.
I will rebase and put in draft.

[codisart]

Hello @boesing should I rebase using #79 also ?

[boesing]

@codisart one step after another. PRs should only rebase against their targets.
Lets wait until we get #79 and/or #75 merged.

[codisart]

@boesing Everything is up to date !

[weierophinney]

@codisart #75 adds PHP 8 support... and, to do so, bumps PHPUnit to 9.3. From what I can see, we're seill using some deprecated assertions currently (though they are not raising notices, from what I can see). As such:

• Please change the title of the PR to "remove deprecated assertion usage"
• Please use $this-> instead of self:: in the patch, for internal consistency; we can revisit $this vs self:: later, and, if we decide on the latter, convert everything at once, instead of piecemeal.

Once those changes are in place, please don't forget to to hit the "Ready for review" button so we can review and merge!

Thanks!

[codisart]

ok let's go !