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.

Sign up for GitHub

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

Jump to bottom

Make each test independent #109

Closed
curquiza opened this issue Feb 15, 2021 · 4 comments
Closed

Make each test independent #109

curquiza opened this issue Feb 15, 2021 · 4 comments
Labels
good first issue Good for newcomers
Projects
SDKs & Integrations

Comments

@curquiza
Copy link
Member

To avoid side effects between each test, the tests should all be independent.

It means, at the beginning of each test:

  • create the needed index(es)
  • push documents according to the test
  • update settings if necessary

And at the end of each test:

  • remove the index(es)

This is linked to #29
@curquiza curquiza added this to Open issues in SDKs & Integrations via automation Feb 15, 2021
@curquiza curquiza mentioned this issue Feb 15, 2021
Merged
@sanders41 sanders41 mentioned this issue Feb 15, 2021
Closed
@sanders41
Copy link
Contributor

sanders41 commented Feb 19, 2021 •
edited

I have looked into creating and deleting the index with each test and the problem I have run into is because the tests are running in threads, the tests still fail. The reason for this is multiple tests can be running at the same time trying to use the same index name. One solution to this is to run the tests all in a single thread with cargo test -- --test-threads=1. By doing this the tests no longer overlap and will pass, but the test take MUCH longer to run.

Some solutions I can currently think of are:

  1. Run all tests in a single thread
    • Advantage: All indexes can have the same name
    • Disadvantage: Slows down the tests
  2. Mock all the network calls
    • Advantages:
      • All indexes can have the same name
      • Tests run fast
    • Disadvantage: Since the main purpose of the client is to talk to Meilisearch through the API moving that to mocks seems like a bad idea.
  3. Give each index a unique name
    • Advantages:
      • Tests run fast
      • Can still use real network calls
    • Disadvantage:
      • Have to ensure that each index name across all files is unique.
      • Doesn't look as good in docs

@Mubelotix
Copy link
Collaborator

@sanders41 I have already thought about this issue. Your third solution has already been applied here and I plan to extend this structure to all other tests. However, there is another issue that you did not expect. For a reason I am unable to explain when tests make simultaneous requests to the MeiliSearch server, requests will hang and never get answers. So we cannot remove test-threads=1 anyway.

@sanders41
Copy link
Contributor

For a reason I am unable to explain when tests make simultaneous requests to the MeiliSearch server, requests will hang and never get answers.

I have seen this issue also. I was guessing it was happening because of the index overlap, but sounds like your testing shows that isn’t the cause.

@curquiza curquiza removed the SDK Rust label Apr 14, 2021
@sanders41 sanders41 mentioned this issue Jun 27, 2021
Closed
@irevoire irevoire mentioned this issue Feb 8, 2022
Merged
bors bot added a commit that referenced this issue Feb 8, 2022
@bors @irevoire
Merge #225
dd3a0a5 
225: Bump meilisearch to v0.25.0 - New task API r=curquiza a=irevoire

We’ll do a little overview of what changed in this PR.

For the users:
- All the update types have been renamed to task
- The creation and deletion of index are now asynchronous
- The `Progress` type have been removed entirely.
- The method `wait_for_pending_update` has been replaced by the method `wait_for_task` which takes a task uid or a task directly in parameter.
- A method `wait_for_completion` has been implemented directly on the Task

For the contributors:
- fix #217 
- #126 has been started with the new `MeiliSearch_test` macro for the unit tests only: https://github.com/meilisearch/meilisearch-rust/blob/41dd6dc765b7938ac399a12db2576b6ab9d479d1/meilisearch-test-macro/README.md
- fix #109

Co-authored-by: Tamo <tamo@meilisearch.com>
@irevoire
Copy link
Member

irevoire commented Feb 9, 2022

Closed by #225

@irevoire irevoire closed this as completed Feb 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue Good for newcomers
Projects
No open projects
SDKs & Integrations
Open issues
Milestone
No milestone
Development

No branches or pull requests
5 participants
@irevoire @curquiza @sanders41 @Mubelotix @meili-bot