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

Error running tests with php7.4 #655

Closed
randykinne opened this issue Nov 17, 2022 · 5 comments
Closed

Error running tests with php7.4 #655

randykinne opened this issue Nov 17, 2022 · 5 comments

Comments

@randykinne
Copy link

Describe the bug

While upgrading our project to use version 3.29, we're encountering the following error in our testing suite:
Class UnitEnum does not exist

After investigating it further, it seems that UnitEnum was added to php in php 8, but we couldn't find anywhere where the library explicitly requires php 8 or in update notes

Steps to reproduce

  1. in php 7.4, load the library and try to use the \Bugsnag\Report class

Environment

  • Bugsnag version: 3.29
  • PHP version: 7.4
  • Phake version: 3.1

Example code snippet

function test_Bugsnag()
{
    $bugsnag_client = Phake::mock(Bugsnag\Client::class);
    $bugsnag_error = Phake::mock(Bugsnag\Report::class);
}
Error messages:
Class UnitEnum does not exist at vendor/phake/phake/src/Phake/ClassGenerator/MockClass.php:546
@chelsea-youmans
Copy link

It seems that it's because of this function declaration:

private function enumToString(UnitEnum $enum)

If you simply remove the UnitEnum parameter type declaration from that line, then Phake will happily mock Bugsnag\Report, even if it's type is still declared in the docblock.

@johnkiely1
Copy link
Member

Hi @randykinne,

The enumToString method using that type isn’t called unless there is a UnitEnum instance in metadata: https://github.com/bugsnag/bugsnag-php/blob/362b93bb4b1318bb4bfe3a9e553413e6c2c1f382/src/Report.php#LL855 so its not clear why Phake would be suggesting this class doesn't exist as it should never actually be called and therefore irrelevant.

The bugsnag-php library is completely type safe and backwards compatible to php5.5. I believe this is more likely an issue with Phake incorrectly catching this as an error. Perhaps you could ask their support team for clarification on this or is there is a suggested workaround.

@johnkiely1 johnkiely1 added the awaiting feedback Awaiting a response from a customer. Will be automatically closed after approximately 2 weeks. label Nov 21, 2022
@chelsea-youmans
Copy link

Hi @johnkiely1!

It isn't so much an issue with being backwards compatible for our general usage of the library, but rather that Phake cannot create a mock of this class because of the strong typing done on enumToString()'s signature.

When you create a mock with Phake, Phake goes and creates mock classes for every type that's referenced in a method signature on the class. This includes private methods, in order to support this feature. So as long as the new UnitEnum type is used on a method signature (even a private one), Phake cannot create a mock for it in older PHP versions.

As I said in my comment above, the solution is to remove UnitEnum on L923. You can leave it in the docblock, but it cannot be used in a method signature in order for Phake to be able to mock it.

@johnkiely1
Copy link
Member

Hi @chelsea-youmans,

I see what you are saying. So in essence because of how Phake mocks classes it can end up creating an implied dependency. To me this does still sound like something Phake should address as it seems like this could cause a lot of false positives across a lot of libraries. Going to close this out as theres not much we can do on it.

Either way it seems you have found a workaround.

@randykinne does this work for you?

@johnkiely1 johnkiely1 removed the awaiting feedback Awaiting a response from a customer. Will be automatically closed after approximately 2 weeks. label Nov 30, 2022
@chelsea-youmans
Copy link

Hi @johnkiely1, I have gone ahead and opened a pull request for the fix I had suggested: #657

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

No branches or pull requests

3 participants