feat(cts): add custom request tests #331
Conversation
✅ Deploy Preview for api-clients-automation canceled.
|
✗ The generated branch has been deleted.If the PR has been merged, you can check the generated code on the |
|
Putting in draft as it need the gen from #330 |
ce404ed to
f36cb98
Compare
f36cb98 to
a56fbab
Compare
| @@ -170,6 +170,9 @@ public Map<String, Object> postProcessSupportingFileData( | |||
| System.out.println(e.getMessage()); | |||
| System.exit(0); | |||
| } | |||
|
|
|||
| System.out.println(e.getMessage()); | |||
| "parameters": { | ||
| "query": "parameters" | ||
| }, | ||
| "body": { |
There was a problem hiding this comment.
DELETE method should not have body, from the http spec:
A payload within a DELETE request message has no defined semantics;
sending a payload body on a DELETE request might cause some existing
implementations to reject the request.
There was a problem hiding this comment.
Good point! It's not the case for the current clients, so I think our API handles it. We can still remove it if we think it shouldn't be the case
There was a problem hiding this comment.
Actually, other DELETE operations we have does not have a request body, so I'll remove it 👍🏼
There was a problem hiding this comment.
I know that algolia API can be non-standard sometimes but I hope they don't break the http specification at least
There was a problem hiding this comment.
I believe we just missed that part in the custom request implementation of the current clients
| @@ -50,8 +50,8 @@ class {{client}}Tests { | |||
|
|
|||
| {{#request.searchParams}} | |||
| HashMap<String, String> expectedQuery = JSON.deserialize("{{#lambda.escapequotes}}{{{request.searchParams}}}{{/lambda.escapequotes}}", new TypeToken<HashMap<String, String>>() {}.getType()); | |||
| List<Pair> acutalQuery = req.getQueryParams(); | |||
| for (Pair p : acutalQuery) { | |||
| List<Pair> actualQuery = req.getQueryParams(); | |||
There was a problem hiding this comment.
oops I have the same code here, you will merge first unfortunately :(
|
no changes in PHP 🤔 |
| @@ -274,6 +274,7 @@ jobs: | |||
| hashFiles( | |||
| format('{0}/lib/Api/{1}.php', matrix.client.folder, matrix.client.api), | |||
| format('{0}/lib/Configuration/{1}.php', matrix.client.folder, matrix.client.config), | |||
| format('{0}/lib/Model/{1}/**', matrix.client.folder, matrix.client.capitalizedName), | |||
There was a problem hiding this comment.
How is that making a difference ? Those files aren't pushed
There was a problem hiding this comment.
Yet they are not, but required by the cache, I wonder if it will work
There was a problem hiding this comment.
An other limitation of the cache :( We will need to add it afterward
scripts/generate.ts
Outdated
| @@ -35,7 +35,7 @@ async function removeExistingModel( | |||
| clientModel = client; | |||
| break; | |||
| case 'php': | |||
| clientModel = createClientName(client, 'php'); | |||
| clientModel = createClientName(client, 'php').replace('Api', ''); | |||
There was a problem hiding this comment.
Good catch ! Instead of using replace can you make a common function to capitalize and have another function to add Api ? This could lead to issues if we have api in the client name
There was a problem hiding this comment.
Actually this function does the right thing, but returns the API name not the client name, I can update it indeed
There was a problem hiding this comment.
The two fns are so rarely used that I'm not even sure which one should be the "base" one, I guess this works: 42ed0a5
🧭 What and Why
🎟 JIRA Ticket:
Changes included:
Follow up of #330
commonfolder for requests common to all clientsifs as it was not triggered🧪 Test
CI :D