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

Mock GoogleAdsClient

Open vienthuong opened this issue 5 years ago • 7 comments

HI there, I got a similar issue like in #294 and the example doesn't help, I wish there is setCredentialsConfig in Google\Ads\GoogleAds\Lib\V3\ServiceClientFactoryTrait so that we can set some mock http client into the client then It won't send the actual request to Google Ads API.

vienthuong avatar May 28 '20 03:05 vienthuong

Pierrick, could you please confirm if this is the same as this one?

fiboknacky avatar May 28 '20 03:05 fiboknacky

Pierrick, could you please confirm if this is the same as this one?

I believe it is strongly related but I do not think it is exactly the same thing:

  • #164 : there is a real connection to a fake but real server that returns canned response via HTTP.
  • Here: my understanding is that the HTTP Client used under the hood would be mocked to return a canned server response without even opening an HTTP connection. Am I getting this right @vienthuong ?

PierrickVoulet avatar May 28 '20 12:05 PierrickVoulet

@PierrickVoulet P Yes that's it! Because the GoogleAdsClient is final class so I cant just mock it, I tried using an adapter but it looks ugly and redundant since it only created for testing purpose.

vienthuong avatar May 28 '20 13:05 vienthuong

Hi, transferred from #639 Why not simply remove "final" keyword from GoogleAdsClient? It solves most of testing problem.

77web avatar Aug 20 '21 11:08 77web

Hi, transferred from #639 Why not simply remove "final" keyword from GoogleAdsClient? It solves most of testing problem.

The reasons are not specific to this library or class, I find them pretty well explained in this article. Note: We do not implement any interface here because there are no public methods, only a constructor.

I never tested this package myself but you may be interested to experiment with it or some other similar solution for your testing.

We are open to discussion, feel free to propose alternatives. There may be other ways to ease testing while keeping the same level of control.

PierrickVoulet avatar Aug 20 '21 12:08 PierrickVoulet

It is possible to edit class & method flags with the extension uopz using the function uopz_flags.

Here is an example where I remove the final flag of the class GoogleAdsClientBuilder and test an extended version of the class with PHPUnit:

$flags = uopz_flags(GoogleAdsClientBuilder::class, null);
uopz_flags(GoogleAdsClientBuilder::class, null, $flags & ~ZEND_ACC_FINAL);

class GoogleAdsClientBuilderExt extends GoogleAdsClientBuilder {
    public function build() {
        return null;
    }
}

class GoogleAdsClientTest extends TestCase
{
    public function test()
    {
        assertNull((new GoogleAdsClientBuilderExt())->build());
    }
}

This can be used to mock GoogleAdsClient or GoogleAdsClientBuilder if you need:

public function test()
{
    $mock = $this->getMockBuilder(GoogleAdsClientBuilder::class)->getMock();
    // ...
}

I am closing this issue but feel free to add comments if you need to follow up.

PierrickVoulet avatar Nov 04 '21 19:11 PierrickVoulet

Hi, transferred from #639 Why not simply remove "final" keyword from GoogleAdsClient? It solves most of testing problem.

The reasons are not specific to this library or class, I find them pretty well explained in this article. Note: We do not implement any interface here because there are no public methods, only a constructor.

If you read Marco's article that you referenced, the very first thing it says is ONLY apply final if they implement an interface. The Google client does not implement an interface. You're going against best practices and making things more difficult for people who want to write quality code and write tests that need access to a dummy client.

If you read Marco's post it very clearly describes

When to avoid final:

Final classes only work effectively under following assumptions:

  1. There is an abstraction (interface) that the final class implements
  2. All of the public API of the final class is part of that interface

If one of these two pre-conditions is missing, then you will likely reach a point in time when you will make the class extensible, as your code is not truly relying on abstractions.

The ONLY correct answer to this issue is final should be removed or the client should have an interface.

bizurkur avatar Apr 02 '22 00:04 bizurkur

Hi there! I'm facing the same issue with GoogleAdsClient. I want to mock it but the final class make it impossible to do so. Is there any possibility to remove final from this class. I understand the meaning of final but it feels like bad practice to use final in libraries like this if there is no interface provided.

UltimateFlex avatar May 24 '23 06:05 UltimateFlex

Fixed with v20.1.0

fiboknacky avatar Aug 10 '23 13:08 fiboknacky

Hi, it seems that "final" has come back for V16 Service Client classes in v22.1.0 unfortunately. They were not marked as final previous versions like V15.

https://github.com/googleads/google-ads-php/tree/main/src/Google/Ads/GoogleAds/V16/Services/Client

They do not have proper interface to implement, so they should not be marked as final.

77web avatar Apr 07 '24 14:04 77web

Edited: Thanks for reporting. It looks like what you reported is different from the previously reported issue.

This one is about making the GoogleAdsClient final, but from what you said, you'd like generated clients to be final too, right? I know that the previously generated classes didn't have the "final" modifier before.

Could you tell me more what's your use case is? Are there any special functionalities that you need to inject into the classes before using them to send a request to the API server?

fiboknacky avatar Apr 08 '24 05:04 fiboknacky