Add Mock API Request Capability and Mocked Connections Tests #314

joshcanhelp · 2019-01-14T21:15:34Z

All changes are in the test suite, no SDK functionality changes.

Add Auth0\Tests\MockApi to generate mocked API endpoints using Guzzle mocks and history
Add Auth0\Tests\MockJsonBody to generate JSON body responses from the mock server
Add Auth0\Tests\API\Management\ConnectionsTestMocked and management-api--connections.json to show how the mocked server is used (pushes Connection coverage to 100% 🎉 )
Change EmailTemplateTest -> EmailTemplatesTest to match endpoint name

lbalmaceda

In general it wouldn't make much sense to have a method that constructs this response for you and you testing the response contents. Because first the combinations or parameters are going to be countless.
And second, you coded this generation logic for the tests sake. You know they are always going to work fine, thus tests would pass and you're not really testing shipped behavior, just "test environment" behavior.

e.g. Why would you test that the generated response did actually filter the items "by strategy" when you know it will succeed because that's how you coded the logic for the response generator? Unless your php implementation (read "no test environment/context") in the API-client-entity-file is doing some extra manipulation after receiving the response. That would be a candidate test case, but if it's a call hand-passing the result it wouldn't.

What it will make sense is to test that the request contains the parameters you want (as you're doing in many of the tests), either in the body or in the query. Also the http method and the full url path. And if you want, test that the response object/array is of type X (e.g. JSON has a "user" schema, ignoring the properties content such as name matches "Juan"). Don't go further than that 👌

lbalmaceda · 2019-01-17T18:26:06Z

tests/API/Management/ConnectionsTestMocked.php

+    protected static $mockJson;
+
+    /**
+     * Instance of MockJsonBody.


lbalmaceda · 2019-01-17T18:36:18Z

tests/API/Management/ConnectionsTestMocked.php

+
+        // Test an explicit true for includeFields.
+        $api->call()->getAll(null, $fields, true);
+        $this->assertContains( 'include_fields=true', $api->getHistoryQuery(1) );


maybe copy this line into the checks for the call right before this one? assertDoesntContain(include_fields)

lbalmaceda · 2019-01-17T18:50:51Z

tests/API/Management/ConnectionsTestMocked.php

+            new Response( 200, self::$headers, self::$mockJson->withPages( 1, 4 )->getClean() ),
+        ] );
+
+        $response1 = $api->call()->getAll(null, null, null, 0, 5);


null, null, null, 0, 5 is getting difficult to follow. Consider defining vars above the call, using named args (if that's a thing for PHP) or passing a Map/Dict instead for the query params and another for the body. GET methods don't get a body, though.

lbalmaceda · 2019-01-17T18:55:52Z

tests/API/Management/ConnectionsTestMocked.php

+        $this->assertArrayHasKey( 'id', $response );
+        $this->assertEquals( $id, $response['id'] );
+        $this->assertEquals( 'GET', $api->getHistoryMethod() );
+        $this->assertEquals( $api::API_BASE_URL.self::ENDPOINT.'/'.$id, $api->getHistoryUrl() );


I'm against using constants references on the tests. What if you change the constant value by mistake adding a typo? Test would still pass. It's fine to use them to initialize the MockApi above but I rather hardcode the full expected path inside these tests in the asserts section e.g. https://myapi.auth0.com/api/v2/connections/con_testConnection10. You'll find it easier to read and follow when you come back as well.

lbalmaceda · 2019-01-17T19:05:23Z

tests/MockApi.php

+     * @param string $endpoint  Endpoint property name to call.
+     * @param array  $responses Responses to be loaded, an array of Response objects.
+     */
+    public function __construct($endpoint, array $responses = [])


consider instead of receiving an array of responses here, having a method like enqueue(response) that would add a single response or an array of responses to the api queue. This way you could instantiate just once the MockApi at the setup step of the test file, since the endpoint is common to the test file, and then just enqueue/write and take/read responses. I think it would simplify a lot the file and would also be more reusable.
You could as well forget about indexes and always keep the "last response". e.g. public api:

MockApi mockApi = new MockApi(endpoint); mockApi.enqueue(response1); mockApi.enqueue(response2); response1 = mockApi.take(); mockApi.lastUrl() == response1.url; response2 = mockApi.take(); mockApi.lastUrl() == response2.url; mockApi.take(); //throws error, queue empty

I tinkered with this quite a bit before I submitted it and, the way it all works, those responses have to be passed to the client before it can be created. I'd either have to do:

$mockApi = new MockApi(endpoint); $mockApi->enqueue(response1); $mockApi->enqueue(response2); $mockApi->build();

... or have enqueue keep track of what's been added so far and re-instantiate the Management API each time. Neither one seemed great. I think the facade for the history API I'm using could be better but this is clear/simple enough for now 👍

Keeping track of the index internally is a great suggestion, thank you.

lbalmaceda · 2019-01-17T19:07:01Z

tests/MockApi.php

+    /**
+     * Return the endpoint being used.
+     *
+     * @return BL | Cl | CG | Co | DC | Em | ET | J | L | R | RS | S | Te | Ti | UB | U | UE


what are these?

Aliases for the classes at the top of the class. They're too long for a single line but without them (@return mixed or something) IDEs won't recognize the method.

Gone now 👍

I think @return line can be multiline. Anyway, never minify classes' names. People shouldn't be trying to guess what you mean. Is fine if you want to add them back instead of removing them, just use the full name. 👌

I think @return line can be multiline.

IDE won't parse it. Trust me, I don't like this either but it retained the functionality needed and is "legal" according to PHP :)

Either way, not needed now.

lbalmaceda · 2019-01-17T19:07:33Z

tests/MockApi.php

+     */
+    public function call()
+    {
+        return $this->client->{$this->endpoint};


why a method named call() would just return the base url / endpoint called? Since this is passed as part of the constructor, it should already be available, known and in scope for whoever is using this MockApi instance, IMO.

I'm now using that to keep track of the history index (see above) but the idea behind it was to give the right endpoint without being able to re-assign it somehow. I altered that a bit to keep the same IDE suggestions and index tracking.

lbalmaceda · 2019-01-17T19:21:37Z

tests/MockJsonBody.php

+        $this->dataBackup = $this->data;
+    }
+
+    /**


Reading from a JSON file is fine. What's not good is having the data being manipulated (filtered, split in pages, etc) here internally and only in the tests environment, because you're adding your own logic that might not be what the server is doing. Either do real api calls or read JSON files with real api responses content.
I'll leave you a general comment detailing this.

Class is gone 👍

lbalmaceda · 2019-01-17T19:23:11Z

tests/API/Management/ConnectionsTestMocked.php

+
+        $this->assertContains( 'page=0', $api->getHistoryQuery() );
+        $this->assertContains( 'per_page=5', $api->getHistoryQuery() );
+        $this->assertCount( 5, $response1 );


what happens when include_totals=true is sent in the query? Can your code parse that JSON structure that the server will send as well? Your method should receive the JSON body and return the expected array, or a "page" object if you don't want the user to loose the page totals.

what happens when include_totals=true is sent in the query?

@joshcanhelp you're still missing this use case I think

@lbalmaceda - Just passes it straight through as JSON, nothing special to test there.

…ginal ConnectionsTest

lbalmaceda

Approved but left one comment! 👍

lbalmaceda · 2019-01-23T20:10:38Z

tests/API/Management/ConnectionsTestMocked.php

+        $this->assertEquals( 'GET', $api->getHistoryMethod() );
+        $this->assertStringStartsWith( 'https://api.test.local/api/v2/connections/'.$id, $api->getHistoryUrl() );
+        $this->assertContains( 'fields='.implode(',', $fields), $api->getHistoryQuery() );
+        $this->assertNotContains( 'include_fields=true', $api->getHistoryQuery() );


This test is supposed to assert that by default this property is not sent, correct? I would check against include_fields (without any value) so that if the property is set to false this would fail as well.

github-actions · 2022-08-23T23:05:09Z

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

joshcanhelp added CH: Added labels Jan 14, 2019

joshcanhelp added this to the v5-Next-Minor milestone Jan 14, 2019

lbalmaceda suggested changes Jan 17, 2019

View reviewed changes

joshcanhelp added 6 commits January 21, 2019 13:01

AddMockApi, MockJsonBody, and minor tweaks in other test classes

dbbdda6

Add Connections mocked tests

5b290d7

API constants cleanup

4628467

Response data cleanup

b0da465

Remove MockJsonBody and result checking; MockApi refactor; delete ori…

e1a7bab

…ginal ConnectionsTest

Delete mock JSON

f976b2e

joshcanhelp force-pushed the add-offline-testing branch from f3d7428 to f976b2e Compare January 21, 2019 21:01

lbalmaceda approved these changes Jan 23, 2019

View reviewed changes

joshcanhelp removed the Ready to Review label Jan 23, 2019

Test tweak

f045971

joshcanhelp merged commit 7c5018d into master Jan 23, 2019

joshcanhelp deleted the add-offline-testing branch January 23, 2019 22:32

github-actions bot locked as resolved and limited conversation to collaborators Aug 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Mock API Request Capability and Mocked Connections Tests #314

Add Mock API Request Capability and Mocked Connections Tests #314

joshcanhelp commented Jan 14, 2019 •

edited

Loading

lbalmaceda left a comment

lbalmaceda Jan 17, 2019

lbalmaceda Jan 17, 2019

lbalmaceda Jan 17, 2019

lbalmaceda Jan 17, 2019

lbalmaceda Jan 17, 2019

joshcanhelp Jan 17, 2019

lbalmaceda Jan 17, 2019

joshcanhelp Jan 17, 2019

joshcanhelp Jan 17, 2019

lbalmaceda Jan 21, 2019

joshcanhelp Jan 21, 2019

lbalmaceda Jan 17, 2019

joshcanhelp Jan 17, 2019

lbalmaceda Jan 17, 2019

joshcanhelp Jan 17, 2019

lbalmaceda Jan 17, 2019

lbalmaceda Jan 21, 2019

joshcanhelp Jan 21, 2019

lbalmaceda left a comment

lbalmaceda Jan 23, 2019

github-actions bot commented Aug 23, 2022

Add Mock API Request Capability and Mocked Connections Tests #314

Add Mock API Request Capability and Mocked Connections Tests #314

Conversation

joshcanhelp commented Jan 14, 2019 • edited Loading

lbalmaceda left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lbalmaceda left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Aug 23, 2022

joshcanhelp commented Jan 14, 2019 •

edited

Loading