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

InvocationMocker annotated as internal, but docs suggest using functions it provides #3742

Closed
bdsl opened this issue Jun 28, 2019 · 8 comments
Labels
feature/test-doubles Stubs and Mock Objects

Comments

@bdsl
Copy link

bdsl commented Jun 28, 2019

Q A
PHPUnit version 8.2.3
PHP version 7.3.5
Installation Method Composer

When using PHPUnit's built-in mocking capabilities, and type-checking my code with Psalm, I get InternalMethod issues raised for PHPUnit\Framework\MockObject\Builder\InvocationMocker::willReturn, PHPUnit\Framework\MockObject\Builder\InvocationMocker::method, and PHPUnit\Framework\MockObject\Builder\InvocationMocker::with.

This is due to the annotation on the class:

* @internal This class is not covered by the backward compatibility promise for PHPUnit

For example with the following code:

    public function test_we_get_null_user_id_for_unknown_user(): void
    {
        $this->mockUserAccountRepository->method('findByAccountId')
            ->with($this->equalTo(Uuid::NIL)) // Psalm emits issue 'The method PHPUnit\Framework\MockObject\Builder\InvocationMocker::with has been marked as internal'
            ->willReturn(null) // Psalm emits issue 'The method PHPUnit\Framework\MockObject\Builder\InvocationMocker::willReturn has been marked as internal'
        ;

        $id = $this->userAccountService->getUserId(Uuid::NIL);

        self::assertSame(null, $id);
    }

But as far as I can tell I'm following the instructions in the docs at https://phpunit.readthedocs.io/en/8.2/test-doubles.html#mock-objects .

If I'm not using PHPUnit wrong, I think that probably either the @internal annotation should be removed from InvocationMocker, and perhaps added to some of its functions that are not mentioned in the documentation, such as the constructor, or the return type declared at

* @method BuilderInvocationMocker method($constraint)
should be changed from BuilderInvocationMocker, which is an alias of InvocationMocker, to an interface containing just the functions we should be relying on.

@sebastianbergmann sebastianbergmann added the feature/test-doubles Stubs and Mock Objects label Jun 28, 2019
@bdsl
Copy link
Author

bdsl commented Jul 9, 2019

@sebastianbergmann Do you have a view on whether this is a PHPUnit bug, and if so what the best way to fix it would be? I'm happy to make a PR if so. Thanks.

@auroraeosrose
Copy link

Workaround for me currently is to add a block (see below) to my psalm.xml but this is annoying - what is the fix? If it's internal are we supposed to use it? If not what are the alternatives?

  <!-- https://github.com/sebastianbergmann/phpunit/issues/3742 - PHPUnit mock objects break all over the place -->
  <InternalMethod>
    <errorLevel type="suppress">
      <referencedMethod name="PHPUnit\Framework\MockObject\Builder\InvocationMocker::willReturn" />
      <referencedMethod name="PHPUnit\Framework\MockObject\Builder\InvocationMocker::method" />
      <referencedMethod name="PHPUnit\Framework\MockObject\Builder\InvocationMocker::willReturnOnConsecutiveCalls" />
      <referencedMethod name="PHPUnit\Framework\MockObject\Builder\InvocationMocker::with" />
      <referencedMethod name="PHPUnit\Framework\MockObject\Builder\InvocationMocker::will" />
      <referencedMethod name="PHPUnit\Framework\MockObject\Builder\InvocationMocker::withConsecutive" />
    </errorLevel>
  </InternalMethod>

@sebastianbergmann
Copy link
Owner

This is an interesting problem related to fluent interfaces, I think.

When you write

$foo = $this->createMock(Foo::class);
$foo->method('bar')->willReturn('baz');

then you rely on the fact that $foo provides method() and whatever is returned by method() provides willReturn(). But you do not care about the actual type, which is an implementation detail, of what is returned by method() -- at least not as long as the test double configuration works.

The good news is that @Ocramius extracted the InvocationStubber interface at this month's code sprint and method() now declares that it returns an instance of InvocationStubber. The InvocationStubber interface declares all the operations that can be performed on what is returned by method().

InvocationStubber is currently annotated with @internal. @Ocramius will correct me if I am wrong, but I do not see a problem with removing the @internal annotation from InvocationStubber.

@Ocramius
Copy link
Sponsor Contributor

Depends on how stable the interface is: if it's good enough, we can drop @internal

@auroraeosrose
Copy link

Thank you @sebastianbergmann and @Ocramius ! Excited to halve my ignores ;)

@lord2800
Copy link

Forgive me for dredging this up, but this still occurs on 8.4.3. Using self::createMock(<anything>)->method('<anything>') still incurs the wrath of InternalMethod. This line seems to be the cause. Removing it locally causes the psalm errors to go away.

@EvgeniiR
Copy link

bump @sebastianbergmann - what about InvocationMocker? Every time I use mocks, I have to put up with calling internal methods, as @lord2800 mentioned above

@sebastianbergmann
Copy link
Owner

8fdb4a6 is in every release since PHPUnit 8.4.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/test-doubles Stubs and Mock Objects
Projects
None yet
Development

No branches or pull requests

6 participants