Odonno / surrealdb-migrations

An awesome SurrealDB migration tool, with a user-friendly CLI and a versatile Rust library that enables seamless integration into any project.
https://crates.io/crates/surrealdb-migrations
MIT License
210 stars 16 forks source link

(integration) tests are flaky #58

Closed turbohz closed 10 months ago

turbohz commented 10 months ago

While implementing https://github.com/Odonno/surrealdb-migrations/pull/56 I noticed that tests will pass sometimes, and then they will not.

When I knew a test should fail, but was passing, I could reliably fix (break again?) the test by restarting the surrealdb instances that are used in the integration tests.

This is probably due to reusing the database, which makes the tests not pure (previous state of the database can influence them).

Ideally each integration test should run in a fresh, uninitialized test database.

From my experience with other languages, they often have hooks to run stuff before/after a test suite or individual tests. I assume something similar can be done in Rust.

turbohz commented 10 months ago

This means, before-integration-tests.sh should not be required to be manually ran.

turbohz commented 10 months ago

I also noticed that sometimes the surrealdb processes crash, or stop for some reason.

Odonno commented 10 months ago

The 2 main goals with tests are:

While implementing https://github.com/Odonno/surrealdb-migrations/pull/56 I noticed that tests will pass sometimes, and then they will not.

Never noticed that. I suppose you have no idea to determine the cause? I think it can happen to have ns/db conflict between 2 tests but that should be like one in a zillion.

Ideally each integration test should run in a fresh, uninitialized test database.

Ideally yes. But then it will take a lot of time to run tests. Apart from the "branches" tests, tests are generally using the same surrealdb instance but in separate ns/db so there should never be any conflict. We still need a right cleanup for "branches" test.

From my experience with other languages, they often have hooks to run stuff before/after a test suite or individual tests. I assume something similar can be done in Rust. This means, before-integration-tests.sh should not be required to be manually ran.

Kind of like setupTest in jest or a beforeAll/afterAll in most languages. Yes, I can definitely see that. Not sure if it is possible at the moment. I mean, I did not found a solution for this.

I also noticed that sometimes the surrealdb processes crash, or stop for some reason.

I never experienced failed test or surrealdb process crash. Are you sure nothing else come in the way? Enough CPU? Enough RAM? I suppose you were not able to identify the cause.

Odonno commented 10 months ago

On another note, I am also interested in a migration to nextest. But that should be in its own PR.

turbohz commented 10 months ago

I see, I never considered that critical that the tests ran fast,

Ideally yes. But then it will take a lot of time to run tests. Apart from the "branches" tests, tests are generally using the same surrealdb instance but in separate ns/db so there should never be any conflict.

I fixed an issue where the integration test was using a db named test, instead of a random named one.

Could be just that one, it might well be, yeah.

I never experienced failed test or surrealdb process crash. Are you sure nothing else come in the way? Enough CPU? Enough RAM? I suppose you were not able to identify the cause.

I work on a beefy desktop PC. Since the two process used are put in the background, I could not catch what happened to them, I just got errors when the test tried to connect.

Could be an issue with my setup, or bugs in surrealdb related with the strict mode, which is what I was testing.

I'll close the issue for now, and reopen if I find out anything more concrete.

Odonno commented 10 months ago

Thank you for your input. It emerges with good idea like splitting test library in 2. Will create an issue for this.