[Feature Request] Warn about useless phpunit assertions

[andreasschroth]

Subject	Details
Issue type	Feature Request
Plugin	Php Inspections (EA Extended), 3.0.6.1
Language level	PHP 7.2

Current behaviour:

This is a feature request, thus the whole check is missing right now.

Expected behaviour:

I sometimes see completely useless assertions in test code, e.g. something like:

$this->assertTrue(true);.

In general, I think such a check should warn if an assertion is completely static. Another example:

$this->assertGreaterThan(2, 5);.

Basically, those checks don't check anything, but just avoid the tests being marked as risky. I often saw some lazy developers add something like $this->assertTrue(true); instead of a proper assertion.

[someniatko]

Consider using "PHPUnit Enhancement" plugin - it has such assertions built-in.

[andreasschroth]

@someniatko Thanks for the info, will check out.

[kalessil]

@someniatko: checked, didn't get any notices for the specified cases. The plugin seems to enhance types resolution in PhpUnit scope, what else =)?

[andreasschroth]

I think a good first step for implementation might be to just check if the passed parameter to the assert*() method is static, i.e. never changes (not being a result of a method call or whatever). Maybe that check is easier to implement than all the logic if an assert check is useful or not.

Because even if the assertion always fails (although that obviously shouldn't ever happen as then the tests would fail and someone should immediately fix), e.g. $this->assertTrue(false);, it should raise a warning.

Anyway, just an idea to get it implemented. =)

[someniatko]

@kalessil @andreasschroth My bad, these inspections are coming from the PHP Inspections EA 🤦‍

[stale[bot]]

Bebo beep, the StaleBot is here. For one year nothing have happened here. It would be great if someone looked into details here within next 21 days when I'll close it.

[kalessil]

Re-opening