Rule php_unit_dedicate_assert should not use assertInternalType anymore.

[drupol]

Hi,

I just noticed that PHP CS Fixer replace this:

-        self::assertTrue(is_resource($file));
+        self::assertInternalType('resource', $file);

However, the method TestCase::assertInternalType does not exist anymore in PHPUnit, it has been deprecated.

On top of that, if just like me you're using the function is_resource to check if a resource is open or not, then replacing it with TestCase::assertInternalType is wrong because it doesn't check that specific thing. Why? Because TestCase::assertInternalType is based on getType() and does not check the status of the resource.

[keradus]

Thanks for the report!

Looks like we have a new feature of assertInternalType -> assertIsResource ;)

I believe it's a quirk that you checked the resources was open, to be honest. Looks like you check the side effect

[keradus]

actually we have that rule already: php_unit_dedicate_assert_internal_type.
how you decided to use php_unit_dedicate_assert only?

[drupol]

Hi Darius,

Actually I was testing if the resource was still open or not, this is why I why using:

self::assertTrue(is_resource($file));

For the rest of your questions, I'll get back to you as soon as possible.

[drupol]

@keradus both rules are existing, I can then use one or another, no ?

[SpacePossum]

if you want this;

-        self::assertTrue(is_resource($file));
+        self::assertIsResource($file);

you'll need both php_unit_dedicate_assert and php_unit_dedicate_assert_internal_type

[drupol]

But that's the thing, this is definitely wrong because it has a different meaning.

When using the function is_resource to check if a resource is open or not, then replacing it with TestCase::assertInternalType is wrong because it doesn't check that specific thing.

TestCase::assertInternalType is based on getType() and does not check the status of the resource.

[SpacePossum]

but replacing with assertIsResource is correct, or also not?

[drupol]

• assertIsResource: assert that a variable is a resource, nothing else. It is based on gettype() only.
• \is_resource(): assert that a variable is a resource (gettype) AND that the resource is open or closed. (https://www.php.net/manual/en/function.is-resource.php)

For me these two are completely different and should not be mixed.

[SpacePossum]

ok, check, I see now 👍
for reference
from;
https://github.com/sebastianbergmann/phpunit/blob/9.5.13/src/Framework/Assert.php#L1519
to;
https://github.com/sebastianbergmann/phpunit/blob/9.5.13/src/Framework/Constraint/Type/IsType.php#L189

not sure how to proceed, no longer do the change, or do the change but update the docs about this, any preferences by anyone?

[drupol]

IMHO: I would just no longer do the change.

[SpacePossum]

sounds good to me, can you prepare a PR for it maybe? let me know if not, no problem than I'll put it on my backlog

[drupol]

I will try... I don't promise you anything.