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

Improve the test macro #238

Open
1 of 2 tasks
irevoire opened this issue Feb 14, 2022 · 6 comments
Open
1 of 2 tasks

Improve the test macro #238

irevoire opened this issue Feb 14, 2022 · 6 comments
Labels
good first issue Good for newcomers

Comments

@irevoire
Copy link
Member

irevoire commented Feb 14, 2022

Currently, our test macro is missing two big features:

  • It should catch all panic and delete the created Index if there was one and then output the panic normally to make the test fail
  • It should be able to understand path in the arguments
@curquiza curquiza added the good first issue Good for newcomers label Feb 14, 2022
@bidoubiwa
Copy link
Contributor

If the index already exists it should still be deleted as we cannot guarantee that the settings are the ones required for the next tests.

To avoid this, we can always delete the index before creating one.
If there is a direct error (no task) it will panic. In case a task is returns but the task fails because the index did not exists, it will be silently ignored.

@irevoire
Copy link
Member Author

Super cool, I think we can keep the issue open, though because deleting the index at the end of the tests is faster. We don't need to wait until the update is processed and can let meilisearch process his tasks.

Also there was a second point in the issue 😁

@bidoubiwa
Copy link
Contributor

Are you suggesting deleting the index only at the end of all the tests or at the end of every test?

It should be able to understand path in the arguments

Oh sorry, I thought it was linked!

bors bot added a commit that referenced this issue Apr 25, 2022
269: Delete index before creation in test macro r=bidoubiwa a=bidoubiwa

Partial fixes of #238 


Co-authored-by: Charlotte Vermandel <[email protected]>
Co-authored-by: cvermand <[email protected]>
@brunoocasali
Copy link
Member

It is not a good thing to delete it after the test because if something happens before the test finishes, we will have an unwanted state. Since we are testing e2e we need to rely on the Meilisearch instance for everything, otherwise, we probably will end with a flaky test.

Good reference about the subject: https://docs.cypress.io/guides/references/best-practices#Using-after-or-afterEach-hooks

@irevoire
Copy link
Member Author

Oooh, interesting. Thanks for the insight!
Currently, the macro deletes the index if no error happens.

@carlosb1
Copy link
Contributor

Related to "understand paths" is the related one for setting up the client like this one?:
"Client: It creates a client like that: Client::new("http://localhost:7700", "masterKey")."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

5 participants