Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(javascript): add requester-testing package #3684

Merged
merged 4 commits into from
Sep 10, 2024

Conversation

shortcuts
Copy link
Member

@shortcuts shortcuts commented Sep 10, 2024

🧭 What and Why

🎟 JIRA Ticket: https://algolia.atlassian.net/browse/DI-2853 https://algolia.atlassian.net/browse/DI-2889

close algolia/algoliasearch-client-javascript#1547

Changes included:

The echo requesters we expose for internal testing purposes contains environment dependant imports (e.g. URL), which lead the clients to be unusable if the user environment doesn't support it

We now have a new private @algolia/requester-testing package, that isn't shipped with the clients and will be used for our test suites, which also allows the @algolia/client-common package to be agnostic.

Jest to Vitest

I've moved the requester-*/client-common/algoliasearch manual tests to vitest, since jest is unable to run esm code (or I didn't managed to?), I wanted to do that since a long time, CTS is next on the list!

The clients/algoliasearch-client-javascript/packages/algoliasearch/__tests__/algoliasearch.common.test.ts suite also runs on the browser bundle now, since the CTS runs on node only, it gives us a bit more safety for potential regressions

@shortcuts shortcuts self-assigned this Sep 10, 2024
@shortcuts shortcuts requested a review from a team as a code owner September 10, 2024 14:13
@algolia-bot
Copy link
Collaborator

algolia-bot commented Sep 10, 2024

✔️ Code generated!

Name Link
🪓 Triggered by c87bfb4043bd1c89130e1820614369af5ca94ac5
🍃 Generated commit 9d6ad4bbcfda5a98cb65b60895280e1d1706b10e
🌲 Generated branch generated/feat/requester-testing-package
📊 Benchmark results

Benchmarks performed on the method using a mock server, the results might not reflect the real-world performance.

Language Req/s
javascript 1658
php 1464
csharp 1261
python 991
java 958
ruby 900
swift 774
go 565

@@ -19,34 +19,29 @@
"types": {
"import": "./dist/common.d.ts",
"module": "./dist/common.d.ts",
"require": "./dist/common.d.cts",
"default": "./dist/common.d.cts"
Copy link
Member Author

Choose a reason for hiding this comment

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

that's a lefover, tsup warns when having a default with import and require saying that it would never be used

Copy link

Copy link
Collaborator

@millotp millotp left a comment

Choose a reason for hiding this comment

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

huuuge

@@ -317,19 +325,22 @@ describe('search with legacy signature', () => {
expect(req.data).toEqual({
requests: [{ indexName: 'theIndexName', hitsPerPage: 42 }],
});
expect(req.searchParams).toStrictEqual(undefined);
expect(req.searchParams).toStrictEqual({
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this change expected ?

Copy link
Collaborator

@millotp millotp left a comment

Choose a reason for hiding this comment

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

gggg

@shortcuts shortcuts merged commit 52e1b57 into main Sep 10, 2024
28 checks passed
@shortcuts shortcuts deleted the feat/requester-testing-package branch September 10, 2024 15:51
algolia-bot added a commit that referenced this pull request Sep 10, 2024
…skip ci]

Co-authored-by: Clément Vannicatte <vannicattec@gmail.com>
algolia-bot added a commit to algolia/algoliasearch-client-javascript that referenced this pull request Sep 10, 2024
algolia/api-clients-automation#3684

Co-authored-by: algolia-bot <accounts+algolia-api-client-bot@algolia.com>
Co-authored-by: Clément Vannicatte <vannicattec@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to resolve module 'url' when using algoliasearch in React Native with Metro bundler
3 participants