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

Expected size is wrong in assertCount error message #3302

Closed
JeroenDeDauw opened this issue Sep 13, 2018 · 13 comments
Closed

Expected size is wrong in assertCount error message #3302

JeroenDeDauw opened this issue Sep 13, 2018 · 13 comments
Labels
type/bug Something is broken

Comments

@JeroenDeDauw
Copy link
Contributor

Q A
PHPUnit version 7.3.5
PHP version 7.2.6
Installation Method Composer

I have this and the test passes:

$this->assertCount( 6, $someGenerator );

I changed it from 6 to 5 to verify the test is being run

$this->assertCount( 5, $someGenerator );

Got error

Failed asserting that actual size 0 matches expected size 5.

While I expected to get

Failed asserting that actual size 6 matches expected size 5.

@sebastianbergmann
Copy link
Owner

I cannot reproduce this:

<?php declare(strict_types=1);
class Test extends PHPUnit\Framework\TestCase
{
    public function testOne(): void
    {
        $a = [1, 2, 3];
        
        $this->assertCount(4, $a);
    }
}
$ phpunit Test
PHPUnit 7.3.5 by Sebastian Bergmann and contributors.

F                                                                   1 / 1 (100%)

Time: 30 ms, Memory: 4.00MB

There was 1 failure:

1) Test::testOne
Failed asserting that actual size 3 matches expected size 4.

/home/sb/Test.php:8

FAILURES!
Tests: 1, Assertions: 1, Failures: 1.

@JeroenDeDauw
Copy link
Contributor Author

My input is a Generator, not an array. I just realized that Generator is not countable, so the "0" in the failure message makes some form of sense. Still, either assertCount should work fully with Generator or not at all.

@sebastianbergmann
Copy link
Owner

Thank you for your report.

Please provide a minimal, self-contained, reproducing test case that shows the problem you are reporting.

Without such a minimal, self-contained, reproducing test case I will not be able to investigate this issue.

@kubawerlos
Copy link
Contributor

    public function testGenerator()
    {
        $generator = static function () {
            yield 1;
            yield 2;
            yield 3;
        };

        static::assertCount(3, $generator());
        static::assertCount(4, $generator());
    }

@sebastianbergmann
Copy link
Owner

  • Generator does not implement Countable
  • Generator implements Iterator
  • Since 4123b6a (to fix PHPUnit and Generators  #2149), PHPUnit iterates over the elements yielded by the generator to count them
  • Due to this iteration, the second assertCount() in the example above fails because the generator yielded all of its elements already

@sebastianbergmann
Copy link
Owner

AFAICS, the only sensible thing to do here would be to disallow assertCount() on generators.

@sebastianbergmann sebastianbergmann added the type/bug Something is broken label Sep 13, 2018
@JeroenDeDauw
Copy link
Contributor Author

The issue does not depend on calling assertCount twice.

   static::assertCount(4, $generator());

That assert alone does the trick. You will get the error saying 0 does not match 4.

@JeroenDeDauw
Copy link
Contributor Author

Disallowing Generator instances would be one way to make the behavior of assertCount consistent, but has the downsides that

  • it is a breaking change
  • assertCount not being able to work with any iterable is unintuitive

@guilliamxavier
Copy link
Contributor

Sorry for bringing more issue rather than a solution, but besides generators, there is the same problem with other non-countable, forward-only (non-rewindable) iterators/traversables, e.g.:

assertCount(4, new \NoRewindIterator(new \ArrayIterator([1, 2, 3])));

or things like PDOStatement (even though a bit far-fetched)...

@JeroenDeDauw
Copy link
Contributor Author

The fix that is up should also take care of those, see #3304

@stale stale bot added the stale label Nov 13, 2018
Repository owner deleted a comment from stale bot Nov 14, 2018
@stale stale bot removed the stale label Nov 14, 2018
@stale
Copy link

stale bot commented Jan 13, 2019

This issue has been automatically marked as stale because it has not had activity within the last 60 days. It will be closed after 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jan 13, 2019
@stale
Copy link

stale bot commented Jan 20, 2019

This issue has been automatically closed because it has not had activity since it was marked as stale. Thank you for your contributions.

@stale stale bot closed this as completed Jan 20, 2019
@epdenouden
Copy link
Contributor

!stale

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug Something is broken
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants