google-ads-php icon indicating copy to clipboard operation
google-ads-php copied to clipboard

Remove final from all classes of this repository

Open gmponos opened this issue 9 months ago • 2 comments

Your client library and Google Ads API versions:

  • Client library version: v23.0.1
  • Google Ads API version: V16

Your environment:

  • The PHP version: 8.3.

Description of the bug:

It has been raised in various issues that since (I think) v21.0 where you marked many classes as final it is a trouble for many developers to mock/test their applications.

You can find other related GH issues here:

https://github.com/googleads/google-ads-php/issues/1008 https://github.com/googleads/google-ads-php/issues/639 https://github.com/googleads/google-ads-php/issues/294 https://github.com/googleads/google-ads-php/issues/347

It is mentioned in these issues that the practice of marking classes as final should be done when you are following some very specific design/coding practices. More info when to mark as final your classes can be found here:

https://ocramius.github.io/blog/when-to-declare-classes-final/

As a result our application is not able to move from v15 to v16 because currently we are unable to mock classes of this package. If we are not able to mock the classes then our Unit tests will end up performing actual HTTP calls over the internet. This has various defects as our pipelines/jobs depend on external resources.

More specifically this is the class that we are trying to mock https://github.com/googleads/google-ads-php/blob/main/src/Google/Ads/GoogleAds/V16/Services/Client/ConversionUploadServiceClient.php but we can't.

I would advise to remove the final from all your classes since the package in many cases does not follow design and coding practices to support having classes as final and it would be troublesome to come back and report each class every time there is a problem with that one.

gmponos avatar May 14 '24 08:05 gmponos