Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove assertions that operate on class/object properties #4601

Closed
sebastianbergmann opened this issue Feb 9, 2021 · 16 comments
Closed

Remove assertions that operate on class/object properties #4601

sebastianbergmann opened this issue Feb 9, 2021 · 16 comments
Assignees
Labels
feature/assertion Issues related to assertions and expectations type/backward-compatibility Something will be/is intentionally broken
Milestone

Comments

@sebastianbergmann
Copy link
Owner

sebastianbergmann commented Feb 9, 2021

Object-Oriented Programming (OOP) is a programming paradigm based on the concept of "objects", which can contain data and code: data in the form of fields (often known as attributes or properties), and code, in the form of procedures (often known as methods).

Wikipedia

PHPUnit currently refers to "fields" (see above) as "attributes". This is (or will become) confusing considering the introduction of attributes in PHP 8 and their support in PHPUnit.

PHPUnit will be changed to use the term "property" instead of "attribute" where "field" is meant.

The following methods of PHPUnit\Framework\Assert will be removed in PHPUnit 10:

  • assertClassHasAttribute()
  • assertClassNotHasAttribute()
  • assertClassHasStaticAttribute()
  • assertClassNotHasStaticAttribute()
  • assertObjectHasAttribute()
  • assertObjectNotHasAttribute()
  • classHasAttribute()
  • classHasStaticAttribute()
  • objectHasAttribute()

The following classes will be removed in PHPUnit 10:

  • PHPUnit\Framework\Constraint\ClassHasAttribute
  • PHPUnit\Framework\Constraint\ClassHasStaticAttribute
  • PHPUnit\Framework\Constraint\ObjectHasAttribute

At this time, no replacements are planned as these assertions are not wildly used (to the best of my knowledge) and, if at all, should only be used in edge case situations that I consider outside the scope of PHPUnit's standard distribution.

@sebastianbergmann sebastianbergmann added type/backward-compatibility Something will be/is intentionally broken feature/assertion Issues related to assertions and expectations labels Feb 9, 2021
@sebastianbergmann sebastianbergmann added this to the PHPUnit 10.0 milestone Feb 9, 2021
@sebastianbergmann sebastianbergmann self-assigned this Feb 9, 2021
@mfn
Copy link

mfn commented Feb 9, 2021

Checked a (private) code base of mine and found a few uses of assertObjectHasAttribute and assertObjectNotHasAttribute

The use-case is when having to work with stdClass (e.g. from JSON deserialization) and there's no notion of what the actual shape is.

@mfn
Copy link

mfn commented Feb 9, 2021

… and indeed, replacing/removing them was a no-brainer 😄

sebastianbergmann added a commit to sebastianbergmann/phpunit-documentation-english that referenced this issue Nov 29, 2021
@sebastianbergmann sebastianbergmann changed the title Deprecate assertions that operate on class/object properties Remove assertions that operate on class/object properties Aug 11, 2022
@uuf6429
Copy link
Contributor

uuf6429 commented Feb 6, 2023

Will there be an alternative replacement or are we supposed to use plain-old-php for this?

-        $this->assertObjectHasAttribute('c', $sut->a->b);
+        $this->assertIsObject($sut->a->b);
+        $this->assertTrue(property_exists($sut->a->b, 'c'));

gbirke added a commit to wmde/fundraising-application that referenced this issue Feb 9, 2023
Remove deprecated `assertObjectHasAttribute` calls.
See sebastianbergmann/phpunit#4601 for the
deprecation decision.

In some cases, we can rely on PHP errors when the attribute is not
present, in other cases (where errors would be very confusing or where
the existence of the property is the only test) we implement our own
check and message.
@florianajir
Copy link

@uuf6429 fixed by #5220 #5231

@imreg
Copy link

imreg commented Mar 8, 2023

Thank you mate, @sebastianbergmann , you made us extra nonsense work for fixing our test environment

@uuf6429
Copy link
Contributor

uuf6429 commented Mar 8, 2023

Thank you mate, @sebastianbergmann , you made us extra nonsense work for fixing our test environment

That's a bit harsh... especially when considering that:

  1. you wouldn't be required to do any changes unless your test environment is on the bleeding edge
  2. if you used the affected methods before, it means that they made sense to you at the time
  3. refactoring code should be a breeze in a decent IDE - PhpStorm, for example, has special handling for deprecations
  4. be grateful that the authors took the time to mark parts as deprecated before actual removal - many other projects do no such thing (you'd be lucky if they follow SemVer)

@lcharette
Copy link

The use-case is when having to work with stdClass (e.g. from JSON deserialization) and there's no notion of what the actual shape is.

Same for XML

@armanist
Copy link

The attribute assertion was really handy thing.. and if the issue only with new PHP attribute features, couldn't it be to rename them, something like?

instead of this:

assertClassHasAttribute()
assertClassNotHasAttribute()

we will have something like this:

assertClassHasProperty()
assertClassNotHasProperty()

@uuf6429
Copy link
Contributor

uuf6429 commented Apr 10, 2023

@armanist please check the following comment a few places up: #4601 (comment)

In short it has been implemented in PHPUnit 10.1, similar to your suggestion.

@joneiros
Copy link

Found an usage of assertObjectNotHasAttribute() in my code.
I realize it doesn't look as nice, but the solution working for my (relatively simple) use case seems to be

$this->assertFalse(property_exists($myObject, $nonExistentPropertyName));

Is there functional reasons NOT to use this replacement approach?

@uuf6429
Copy link
Contributor

uuf6429 commented Apr 22, 2023

@joneiros There are some minor differences:

  • It doesn't check that the object is indeed an object (and you might get a type error if it wasn't)
  • If the assertion fails, you'll get a generic "value expected to be false, but it is true" message. This can be fixed by passing your own message as the 2nd parameter though.

Anyway, you can see all that by checking the phpunit implementation (the deprecated version or the new one, linked above).

(in short and in my opinion, I'd keep using the one provided by phpunit)

@sebastianbergmann may I suggest updating the description of the issue to say that there will be in fact a replacement, to avoid more "me too" comments?

sashas777 added a commit to sashas777/magento2-testing-framework that referenced this issue May 12, 2023
@AnjaLiebermann
Copy link

@armanist please check the following comment a few places up: #4601 (comment)

In short it has been implemented in PHPUnit 10.1, similar to your suggestion.

That's fine and dandy, but currently I get the warning with the hint to replace it with a method, which doesn't exist yet.
I am using PHPUnit 9.6.8

@FlorientR
Copy link

Hi !
So if I use PHPUnit 9.6.8 on PHP 8.0, I will always have the warning message .... What solution for don't have warning message ?

@stronk7
Copy link
Contributor

stronk7 commented Jul 24, 2023

So if I use PHPUnit 9.6.8 on PHP 8.0, I will always have the warning message .... What solution for don't have warning message ?

Stay sticky to PHPUnit 9.6.0 (the warnings were introduced in 9.6.1, AFAIK). It's really unfortunate that the new alternatives (assertObjectHasProperty() ... have not been also backported to the 9.x series, grrr, coz that makes the warning 100% useless.

Ciao :-)

@swiffer
Copy link

swiffer commented Aug 22, 2023

@stronk7 - they have been backported in 9.6.11 ;)

https://github.com/sebastianbergmann/phpunit/blob/9.6.11/ChangeLog-9.6.md

@stronk7
Copy link
Contributor

stronk7 commented Aug 22, 2023

Yeah, @swiffer, I was following the other issue, awesome!

Problem for us is that we also faced a similar problem with other deprecation messages (for example, the removal in PHPUnit 10 of expect* methods, and the deprecation message in 9.6.x without replacement) that would hit us (and all our plugins legion) if we jump to 9.6.x suddenly.

Still analysing if to add PHPUnit Polyfills may be a possible solution or we'll stay sticky to 9.5.x for the life of our released (stable) branches.

Ciao :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/assertion Issues related to assertions and expectations type/backward-compatibility Something will be/is intentionally broken
Projects
None yet
Development

No branches or pull requests