Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
0f2050b
chore: add client testing
eunjae-lee Jan 14, 2022
edeed9f
chore: skip when template is missing
eunjae-lee Jan 14, 2022
62eec79
chore: skip when tests for client don't exist
eunjae-lee Jan 14, 2022
5079f25
test region WIP
eunjae-lee Jan 19, 2022
2c2a49e
fix tests
eunjae-lee Jan 20, 2022
0ce50ab
remove unnecessary url from echo requester
eunjae-lee Jan 20, 2022
3b3dfa8
Merge branch 'main' into chore/integration
eunjae-lee Jan 20, 2022
823b3dd
Merge branch 'main' into chore/integration
Jan 20, 2022
4f1cd88
fix: update template to check region
eunjae-lee Jan 21, 2022
38c6dd4
Merge branch 'chore/integration' of github.com:algolia/api-clients-au…
eunjae-lee Jan 21, 2022
935f934
Merge branch 'main' into chore/integration
eunjae-lee Jan 21, 2022
f784d23
fix: add hasReigonalHost to generators
eunjae-lee Jan 21, 2022
29bc84c
fix: make regional optional in client testing
eunjae-lee Jan 21, 2022
65ab886
fix: test asynchronous errors
eunjae-lee Jan 21, 2022
bb63439
Merge branch 'main' into chore/integration
Jan 21, 2022
5f31dd3
update tests
eunjae-lee Jan 21, 2022
587a222
Merge branch 'chore/integration' of github.com:algolia/api-clients-au…
eunjae-lee Jan 21, 2022
46b535e
fix: remove duplicated import statements
eunjae-lee Jan 21, 2022
180817d
chore: remove unused part
eunjae-lee Jan 24, 2022
a7c9514
chore: format cts output
eunjae-lee Jan 24, 2022
93b385b
Merge branch 'main' into chore/integration
eunjae-lee Jan 24, 2022
04d4e4a
Merge branch 'main' into chore/integration
Jan 24, 2022
26cc9ca
fix: remove createIndex
eunjae-lee Jan 25, 2022
2b5871b
chore: do not use post- script in package.json
eunjae-lee Jan 25, 2022
971cebf
fix: type issues
eunjae-lee Jan 25, 2022
794326f
Update tests/CTS/client/templates/javascript/suite.mustache
Jan 25, 2022
69e2983
chore: add eslint to tests
eunjae-lee Jan 25, 2022
575d88c
Merge branch 'chore/integration' of github.com:algolia/api-clients-au…
eunjae-lee Jan 25, 2022
c78aadc
chore: update output
eunjae-lee Jan 26, 2022
39f8503
Merge branch 'main' into chore/integration
eunjae-lee Jan 26, 2022
7a0358d
run java cts on the CI
millotp Jan 26, 2022
169d190
Merge branch 'main' into chore/integration
Jan 26, 2022
d7a6955
Revert "run java cts on the CI"
millotp Jan 26, 2022
b0fca35
Merge branch 'chore/integration' of github.com:algolia/api-clients-au…
millotp Jan 26, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,16 @@ export class AbtestingApi {
region: 'de' | 'us',
options?: { requester?: Requester; hosts?: Host[] }
) {
if (!appId) {
throw new Error('`appId` is missing.');
}
if (!apiKey) {
throw new Error('`apiKey` is missing.');
}
if (!region) {
throw new Error('`region` is missing.');
}

this.setAuthentication({ appId, apiKey });

this.transporter = new Transporter({
Expand All @@ -72,7 +82,7 @@ export class AbtestingApi {
});
}

getDefaultHosts(region: 'de' | 'us' = 'us'): Host[] {
getDefaultHosts(region: 'de' | 'us'): Host[] {
return [
{
url: `analytics.${region}.algolia.com`,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,13 @@ export class AnalyticsApi {
region: 'de' | 'us',
options?: { requester?: Requester; hosts?: Host[] }
) {
if (!appId) {
throw new Error('`appId` is missing.');
}
if (!apiKey) {
throw new Error('`apiKey` is missing.');
}

this.setAuthentication({ appId, apiKey });

this.transporter = new Transporter({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,13 @@ export class InsightsApi {
apiKey: string,
options?: { requester?: Requester; hosts?: Host[] }
) {
if (!appId) {
throw new Error('`appId` is missing.');
}
if (!apiKey) {
throw new Error('`apiKey` is missing.');
}

this.setAuthentication({ appId, apiKey });

this.transporter = new Transporter({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,16 @@ export class PersonalizationApi {
region: 'eu' | 'us',
options?: { requester?: Requester; hosts?: Host[] }
) {
if (!appId) {
throw new Error('`appId` is missing.');
}
if (!apiKey) {
throw new Error('`apiKey` is missing.');
}
if (!region) {
throw new Error('`region` is missing.');
}

this.setAuthentication({ appId, apiKey });

this.transporter = new Transporter({
Expand All @@ -72,7 +82,7 @@ export class PersonalizationApi {
});
}

getDefaultHosts(region: 'eu' | 'us' = 'us'): Host[] {
getDefaultHosts(region: 'eu' | 'us'): Host[] {
return [
{
url: `personalization.${region}.algolia.com`,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,16 @@ export class QuerySuggestionsApi {
region: 'eu' | 'us',
options?: { requester?: Requester; hosts?: Host[] }
) {
if (!appId) {
throw new Error('`appId` is missing.');
}
if (!apiKey) {
throw new Error('`apiKey` is missing.');
}
if (!region) {
throw new Error('`region` is missing.');
}

this.setAuthentication({ appId, apiKey });

this.transporter = new Transporter({
Expand All @@ -74,7 +84,7 @@ export class QuerySuggestionsApi {
});
}

getDefaultHosts(region: 'eu' | 'us' = 'us'): Host[] {
getDefaultHosts(region: 'eu' | 'us'): Host[] {
return [
{
url: `query-suggestions.${region}.algolia.com`,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,13 @@ export class SearchApi {
apiKey: string,
options?: { requester?: Requester; hosts?: Host[] }
) {
if (!appId) {
throw new Error('`appId` is missing.');
}
if (!apiKey) {
throw new Error('`apiKey` is missing.');
}

this.setAuthentication({ appId, apiKey });

this.transporter = new Transporter({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,13 @@ export class RecommendApi {
apiKey: string,
options?: { requester?: Requester; hosts?: Host[] }
) {
if (!appId) {
throw new Error('`appId` is missing.');
}
if (!apiKey) {
throw new Error('`apiKey` is missing.');
}

this.setAuthentication({ appId, apiKey });

this.transporter = new Transporter({
Expand Down
1 change: 1 addition & 0 deletions openapitools.json
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@
"packageName": "@algolia/client-analytics",
"hasRegionalHost": true,
"isDeHost": true,
"fallbackToUS": true,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only analytics client falls back to us region.

"host": "analytics"
}
},
Expand Down
16 changes: 15 additions & 1 deletion templates/javascript/api-single.mustache
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,20 @@ export class {{classname}} {
{{/hasRegionalHost}}
options?: {requester?: Requester, hosts?: Host[]}
) {
if (!appId) {
throw new Error("`appId` is missing.");
}
if (!apiKey) {
throw new Error("`apiKey` is missing.");
}
{{#hasRegionalHost}}
{{^fallbackToUS}}
if (!region) {
throw new Error("`region` is missing.");
}
{{/fallbackToUS}}
{{/hasRegionalHost}}

this.setAuthentication({ appId, apiKey });

this.transporter = new Transporter({
Expand Down Expand Up @@ -96,7 +110,7 @@ export class {{classname}} {

{{^isSearchHost}}
{{#hasRegionalHost}}
public getDefaultHosts(region: {{#isDeHost}}'de'{{/isDeHost}}{{#isEuHost}}'eu'{{/isEuHost}} | 'us' = 'us'): Host[] {
public getDefaultHosts(region: {{#isDeHost}}'de'{{/isDeHost}}{{#isEuHost}}'eu'{{/isEuHost}} | 'us'{{#fallbackToUS}} = 'us'{{/fallbackToUS}}): Host[] {
return [{ url: `{{{host}}}.${region}.algolia.com`, accept: 'readWrite', protocol: 'https' }];
}
{{/hasRegionalHost}}
Expand Down
33 changes: 33 additions & 0 deletions tests/CTS/client/analytics/basic.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
[
{
"testName": "does not throw when region is not given",
"autoCreateClient": false,
"steps": [
{
"type": "createClient",
"parameters": {
"appId": "my-app-id",
"apiKey": "my-api-key",
"region": ""
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing the region can be tricky, because in typescript this won't even compile because of the union but in other languages it's just a string so it's fine, or enum.
Maybe the javascript template can skip test where it nows it won't compile, like having a tag or something.

Copy link
Contributor Author

@eunjae-lee eunjae-lee Jan 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, but if I were to write this test case manually, I'd do something like:

expect(() => {
  // @ts-expect-error it's expecting `region` but intentionally miss it to make it throw
  createClient({ appId: '...', apiKey: '...' });
}).toThrow(....);

So I think it's alright to have that test, but we need to way to tell TS that it's intentional.

Maybe we could update the test template like

await expect(new Promise((resolve, reject) => {
{{> step}}

                await expect(new Promise((resolve, reject) => {
+                   // @ts-ignore
                    {{> step}}

What do you think?

edit:

but // @ts-ignore is not enough.

        // @ts-expect-error this is expected
        const $client = new AnalyticsApi(
          'app-id',
          'api-key',
          '', // <- region
          {
            requester: new EchoRequester(),
          }
        );

because we need to ignore the whole block, but it's not supported by TS: microsoft/TypeScript#19573

And I'm not sure how much we need to invest on types in test code. To pass the CI, I put this. We can go with this for now, and improve it progressively.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use // @ts-expect-error above the line to make the compilation work

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can remove some code by removing the async await, for example:

describe('basic', () => {
  test('does not throw when region is not given', () => {
    expect(() => {
      // @ts-expect-error
      // eslint-disable-next-line no-new
      new AnalyticsApi('my-app-id', 'my-api-key', '', {
        requester: new EchoRequester(),
      });
    }).not.toThrow();
  });

  test('getAverageClickPosition throws without index', () => {
    const $client = createClient();
    // @ts-expect-error
    expect(() => $client.getClickPositions({})).toThrow(
      'Parameter `index` is required when calling `getClickPositions`.'
    );
  });
});

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah but // @ts-expect-error works for only the next line, and there's no guarantee.

  1. Removing some of async / await is hard, unless we specify that this methid is async in the test json file.

For example,

      {
        "type": "method",
        "object": "$client",
        "path": "getClickPositions",
+       "async": true,
        "parameters": [{}],
        "expected": {
          "error": "Parameter `index` is required when calling `getClickPositions`."
        }
      }

We could do this, but this async is not a thing in other many languages. But we could still add it to make things easier for JS, and let other languages ignore it.

However,

  1. What if the line gets longer and linter breaks it down to multiple lines? Then // @ts-expect-error cannot work. That's why I googled if it's possible to ignore a certain block of code, instead of next line. (which is not possible at the moment)

},
"expected": {
"error": false
}
}
]
},
{
"testName": "getAverageClickPosition throws without index",
"steps": [
{
"type": "method",
"object": "$client",
"path": "getClickPositions",
"parameters": [{}],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same thing here for typescript, this won't compile, but will in some languages

"expected": {
"error": "Parameter `index` is required when calling `getClickPositions`."
}
}
]
}
]
17 changes: 17 additions & 0 deletions tests/CTS/client/search/basic.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
[
{
"testName": "client throws with invalid parameters",
"autoCreateClient": false,
"steps": [
{
"type": "createClient",
"parameters": {
"apiKey": "blah"
},
"expected": {
"error": "`appId` is missing."
}
}
]
}
]
8 changes: 8 additions & 0 deletions tests/CTS/client/templates/javascript/createClient.mustache
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
new {{client}}(
'{{parameters.appId}}',
'{{parameters.apiKey}}',
{{#hasRegionalHost}}'{{parameters.region}}',{{/hasRegionalHost}}
{
requester: new EchoRequester()
}
)
3 changes: 3 additions & 0 deletions tests/CTS/client/templates/javascript/expected.mustache
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{{#length}}
expect(actual).toHaveLength({{length}});
{{/length}}
1 change: 1 addition & 0 deletions tests/CTS/client/templates/javascript/method.mustache
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{{object}}{{#path}}.{{.}}{{/path}}({{{parameters}}});
12 changes: 12 additions & 0 deletions tests/CTS/client/templates/javascript/step.mustache
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{{#isCreateClient}}
const $client = {{> createClient}}
actual = $client;
{{/isCreateClient}}

{{#isVariable}}
actual = {{> variable}}
{{/isVariable}}

{{#isMethod}}
actual = {{> method}}
{{/isMethod}}
65 changes: 65 additions & 0 deletions tests/CTS/client/templates/javascript/suite.mustache
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/* eslint-disable @typescript-eslint/no-unused-vars */
/* eslint-disable require-await */
/* eslint-disable @typescript-eslint/explicit-function-return-type */
// @ts-nocheck
import { {{client}}, EchoRequester } from '{{{import}}}';

const appId = process.env.ALGOLIA_APPLICATION_ID || 'Algolia-API-Key';
const apiKey = process.env.ALGOLIA_SEARCH_KEY || 'Algolia-Application-Id';

function createClient(): {{client}} {
return new {{client}}(appId, apiKey, {{#hasRegionalHost}}'us', {{/hasRegionalHost}}{ requester: new EchoRequester() });
}

{{#blocks}}
describe('{{operationId}}', () => {
{{#tests}}
test('{{testName}}', async () => {
{{#autoCreateClient}}
const $client = createClient();
{{/autoCreateClient}}

let actual;
{{#steps}}
{{#expectedError}}
await expect(new Promise((resolve, reject) => {
{{> step}}
if (actual instanceof Promise) {
actual.then(resolve).catch(reject);
} else {
resolve();
}
})).rejects.toThrow("{{{expectedError}}}")
{{/expectedError}}
Comment on lines +24 to +33
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We promisify the return of the step and expects it to throw.


{{^expectedError}}
{{#expectedNoError}}
await expect(new Promise((resolve, reject) => {
{{> step}}
if (actual instanceof Promise) {
actual.then(resolve).catch(reject);
} else {
resolve();
}
})).resolves.not.toThrow();
{{/expectedNoError}}

{{^expectedNoError}}
{{> step}}

if (actual instanceof Promise) {
actual = await actual;
}
Comment on lines +50 to +52
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The return of the step can be a promise, so we resolve it and then move onto validating the result.


{{#expected}}
{{> expected}}
{{/expected}}
{{/expectedNoError}}
{{/expectedError}}
{{/steps}}
});

{{/tests}}
})

{{/blocks}}
1 change: 1 addition & 0 deletions tests/CTS/client/templates/javascript/variable.mustache
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{{object}}{{#path}}.{{.}}{{/path}};
File renamed without changes.
50 changes: 50 additions & 0 deletions tests/output/javascript/tests/client/analytics.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// @ts-nocheck
import { AnalyticsApi, EchoRequester } from '@algolia/client-analytics';

const appId = process.env.ALGOLIA_APPLICATION_ID || 'Algolia-API-Key';
const apiKey = process.env.ALGOLIA_SEARCH_KEY || 'Algolia-Application-Id';

function createClient(): AnalyticsApi {
return new AnalyticsApi(appId, apiKey, 'us', {
requester: new EchoRequester(),
});
}

describe('basic', () => {
test('does not throw when region is not given', async () => {
let actual;

await expect(
new Promise((resolve, reject) => {
const $client = new AnalyticsApi('my-app-id', 'my-api-key', '', {
requester: new EchoRequester(),
});
actual = $client;

if (actual instanceof Promise) {
actual.then(resolve).catch(reject);
} else {
resolve();
}
})
).resolves.not.toThrow();
});

test('getAverageClickPosition throws without index', async () => {
const $client = createClient();

let actual;
await expect(
new Promise((resolve, reject) => {
actual = $client.getClickPositions({});
if (actual instanceof Promise) {
actual.then(resolve).catch(reject);
} else {
resolve();
}
})
).rejects.toThrow(
'Parameter `index` is required when calling `getClickPositions`.'
);
});
});
Loading