search

Brewtarget / brewtarget

Main brewtarget source code repository.
GNU General Public License v3.0
312 stars 135 forks source link

TESTs failing when Testing::initTestCase() is creating new Hop or Fermentable #604

Closed mattiasmaahl closed 3 years ago

mattiasmaahl commented 3 years ago

So I have been going though the debugging for the test and found that when the cascade_4pct = Database::instance().newHop(); or the twoRow = Database::instance().newFermentable(); gets called the tests fail.

I have narrowed it down to the call to return Database::instance().modifyEntry(this, prop_name, value); in NamedEntity.cpp:390

A theory is that when we are running the tests Brewtarget tries to add entries to the inventory, but that is just a theory at this point, I'm still investigating how, according to my limited mind, we do this best in the testing scenario.

matty0ung commented 3 years ago

Do we use a different copy of the DB file for testing?

mattiasmaahl commented 3 years ago

I don't think we do actually, or rather, we may do as we set the "application name" to be "brewtarget-test" that should store all application files in its own folder, but I have never seen the db there. Could be that wen may have to do a restore default to get a new/fresh db there.

I'll have a look.

mattiasmaahl commented 3 years ago

Yeah so I checked, there is a database.sqlite created in the directory. So we are running the tests to a "restored" default_db.sqlite database

EDIT: remembered the default db file name wrong. :smile:

mattiasmaahl commented 3 years ago

so here are the logs from where it crashes in the tests: [11:22:21.132] DEBUG : Creating new Hop: cascade_4pct = Database::instance().newHop();, in 93 [11:22:21.132] DEBUG : T Database::newNamedEntity(QHash<int, T>) [with T = Hop] SELECT SQL: "SELECT FROM hop WHERE id = 136", in 172 [11:22:21.132] DEBUG : Recipe Database::getParentRecipe(const NamedEntity) NamedEntity in recipe search: "SELECT recipe_id AS id FROM hop_in_recipe WHERE hop_id=136", in 1116 [11:22:21.132] ERROR : void Database::updateEntry(NamedEntity, QString, QVariant, bool, bool) Could not translate "inventoryId" to a column name, in 3118 [11:22:21.132] ERROR : "Hop Database::newHop(Hop*, bool) Could not translate inventoryId to a column name", in 1744

So there is a thing with the inventoryId. I just checked that the default_db.sqlite also has the inventory_id column in the Hop Table, so something is happening in the code? @matty0ung any thoughts on this? you and @mikfire are the ones more read up on these parts of the code.

Also, I may be way out on a wild goose chase deep in parts where there is no error and the solution probably is simple. if so, don't be afraid do call me out on it! :smile:

matty0ung commented 3 years ago

Ah, interesting. Yes, I wouldn't have seen this in Brewken because of the different database layer. What happens if you edit line 87 of TableSchemaConst.h to say "inventoryId" instead of "inventory_id"? (Or, failing that, change "inventoryId" to "inventory_id" on line 26 of model/NamedEntityWithInventory.h.)

mattiasmaahl commented 3 years ago

Interesting, I will check in just a moment.

mattiasmaahl commented 3 years ago

So tested both ways and found that both solutions work.

do you have a preferred solution given that this somewhat plays into your work with DB overhaul?