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 problematic test case #328

Merged

Conversation

jseparovic1
Copy link
Contributor

@jseparovic1 jseparovic1 commented Mar 7, 2024

Reverts only part of #316

Following test case produces different phpstan analyisis output depending on used versions:

  public function testCreateProphecyInline(): void
    {
        $prophecy = (new Prophet())->prophesize(Src\BaseModel::class);

        $prophecy
            ->getFoo()
            ->willReturn('bar');

        $prophecy
            ->doubleTheNumber(Argument::is(2))
            ->willReturn(5);

        $testDouble = $prophecy->reveal();

        self::assertEquals('bar', $testDouble->getFoo());
        self::assertEquals(5, $testDouble->doubleTheNumber(2));
    }

Creating Prophet inline seems like a valid use case that should be supported as well. In combination with PHPUnit 10.0.0. this produced internal phpstan error due to following lines. I added the test case thus this use case is visible and supported however output is different based on combination of versions thus it is hard to support this without having version specific baseline(s).

More developers are reaching for this use case as PHPUnit 10 deprecated non-static data providers (see sebastianbergmann/phpunit@9caafe2). Easy way out is to use (new Prophet()) .

@jseparovic1 jseparovic1 force-pushed the fix-build-fails-due-to-wrong-baseline branch from bcf1005 to 81e1b4f Compare March 7, 2024 17:08
@jseparovic1
Copy link
Contributor Author

@alexander-schranz As per discussion in #285 I opted to use approach that includes minimal changes to make the build green. Let me know what do you think :)

@jseparovic1 jseparovic1 changed the title Fix build by generating new baseline Remove problematic test case Mar 8, 2024
@ivan-wolf
Copy link

This replaces #327, correct?

@alexander-schranz alexander-schranz merged commit dfa3e26 into Jan0707:master Mar 13, 2024
14 checks passed
@alexander-schranz
Copy link
Collaborator

Fine for me currently, is there still a way we could test this?

@jseparovic1 jseparovic1 deleted the fix-build-fails-due-to-wrong-baseline branch March 13, 2024 09:59
@jseparovic1
Copy link
Contributor Author

jseparovic1 commented Mar 13, 2024

@alexander-schranz Test case described in the issue is one that is verifying this functionality and it should be included (sooner or later). Currently only limitation is different phpstan output which we could add and create CI matrix to test it as well. Moreover if support is dropped for older PHP versions output might be more consistent.

Jean85 added a commit to facile-it/paraunit that referenced this pull request Mar 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants