native_db icon indicating copy to clipboard operation
native_db copied to clipboard

DatabaseConnectionFailed->RedbDatabaseError->DatabaseAlreadyOpen during testing

Open aramrw opened this issue 11 months ago • 3 comments

[cfg(test)]
mod db_tests {
    use crate::{database::dictionary_database::Queries, Yomichan};
    use pretty_assertions::assert_eq;

    #[test]
    fn lookup_exact() {
        let ycd = Yomichan::new("./a/yomichan/data.yc").unwrap();
        let res = ycd.lookup_exact("日本語").unwrap();
        let first = res.first().unwrap();
        assert_eq!(
            ("日本語", "にほんご"),
            (&*first.expression, &*first.reading)
        )
    }

    #[test]
    fn bulk_lookup_term() {
        let ycd = Yomichan::new("./a/yomichan/data.yc").unwrap();
        let res = ycd.bulk_lookup_term(Queries::Exact(&["日本語"])).unwrap();
        let first = res.first().unwrap();
        assert_eq!(("日本語", "にほんご"), (&*first.term, &*first.reading))
    }
}

They both pass and fail, depending on which one comes first.

running 2 tests
- test database::handlers::db_tests::lookup_exact ... FAILED
+ test database::handlers::db_tests::bulk_lookup_term ... ok

failures:
---- database::handlers::db_tests::lookup_exact stdout ----
- thread 'database::handlers::db_tests::lookup_exact' panicked at src\database\handlers.rs:434:57:
called `Result::unwrap()` on an `Err` value: DatabaseConnectionFailed(RedbDatabaseError(DatabaseAlreadyOpen))
running 2 tests
- test database::handlers::db_tests::bulk_lookup_term ... FAILED
+ test database::handlers::db_tests::lookup_exact ... ok

failures:

---- database::handlers::db_tests::bulk_lookup_term stdout ----
- thread 'database::handlers::db_tests::bulk_lookup_term' panicked at src\database\handlers.rs:445:57:
called `Result::unwrap()` on an `Err` value: DatabaseConnectionFailed(RedbDatabaseError(DatabaseAlreadyOpen))

Why is the database still open when this function returns? Shouldnt it have been dropped and closed by the time bulk_lookup_term() is called in the test? And vice versa. I'm not sure what I'm doing wrong or how to fix it

/// Queries multiple terms via text.
pub fn lookup_exact<Q: AsRef<str> + Debug>(&self, query: Q) -> Result<VecDBTermEntry, DBError> {
        let query = query.as_ref();
        let db = DBBuilder::new().open(&DB_MODELS, &self.db_path)?;
        let rtx = db.r_transaction()?;

        let mut exps = query_sw(&rtx, DatabaseTermEntryKey::expression, query)?;
        let readings = query_sw(&rtx, DatabaseTermEntryKey::reading, query)?;

        if exps.is_empty() && readings.is_empty() {
            return Err(DBError::Query(format!("no entries found for: {}", query)));
        }

        exps.extend(readings);
        Ok(exps)
}
fn query_sw<'a, R: native_db::ToInput>(
    rtx: &RTransaction,
    key: impl ToKeyDefinition<KeyOptions>,
    query: impl ToKey + 'a,
) -> Result<Vec<R>, DBError> {
    Ok(rtx
        .scan()
        .secondary(key)?
        .start_with(query)
        .collect::<Result<Vec<R>, db_type::Error>>()?)
}

Thank you for the help

aramrw avatar Jan 26 '25 22:01 aramrw

@aramrw how do you run your tests?

Rust tests are executed by default in parallel. Try running them with this parameter cargo test -- --test-threads=1.

However, the most common solution is to use a temporary directory that generates a random name for your db, see tempfile.

vincent-herlemont avatar Jan 27 '25 09:01 vincent-herlemont

Thanks 👍🏼

aramrw avatar Jan 28 '25 04:01 aramrw

Sorry for closing then reopening, but after going over the code again I just realized that since I'm purely reading from the db (prev example) shouldn't it still work even if multiple threads have the file open? I just checked old tests, and I was even writing to database between threads (without sharing the db instance)...?

let mut dictionary_options: Vec<DictionaryOptions> = zip_paths
            .par_iter()
            .map(|path| import_dictionary(path, settings, db))
            .collect::<Result<Vec<DictionaryOptions>, DBError>>()?;

/// Commented out lines are from 6 months ago
/// I passed the path and opened the db in each seperate thread. How...?
pub fn import_dictionary<P: AsRef<Path>>(
    //db_path: &OsString,
    db: &Database,
) -> Result<DictionaryOptions, DBError> {
    let data: DatabaseDictData = prepare_dictionary(zip_path, settings)?;
    // let db = DBBuilder::new().open(&DB_MODELS, db_path)?;
    let rwtx = db.rw_transaction()?;

I had to refactor this now to share the db instance across the struct instead of opening the db file directly.

This is a much better way to do things imo, but i haven't touched this codebase in 6 months (until now to get it working), and i still have the working db generated from the previous code, so I can garuntee this was possible.

I'm wondering if that was a bug, or maybe this is a bug? or maybe I am misunderstanding. I also want to say all my code is working after some changes 👍🏼 so im just reopening out of curiousity

aramrw avatar Jan 28 '25 07:01 aramrw