Fix tests

[VRehnberg]

Solves #1054

I'm actually only using one of the example config.ini files, but I figured I'd include both with and without AI for that specific version to make it easier for future testing.

[VRehnberg]

One had not been updated since #1024 and the other tried to access self.settings, but that had only been saved to App.settings so changed that.

[ccbogel]

OK thank you - I have read through the changes you have made to learn from it.

[kaixxx]

Thank you for implementing these test, Viktor!
One thing worries me a bit: The handling of the config.ini is already quite complicated, and making changes requires adjustments in several places of the code. Now, the test suite adds another layer that we must keep up to date if me make any changes to the config.ini. Wouldn't it be better to use App.default_settings() to create a new config on the fly during the tests? This way, we only need to update this function if we want to add new settings.
The same idea could be applied to the database: Why not use App.new_project() to create a new db with the most recent structure instead of creating all the tables manually?

[VRehnberg]

@kaixxx I agree.

Didn't see the App.default_settings method. That should probably be used in this case, my bad.

The check for expected keys in config.ini I tried to keep as close as possible to as it was, but as is the value from it is quite low to the maintenance cost of keeping the CONFIG_INI constants up to date.

[ccbogel]

That would be my fault, based on what I originally did, when trying to work out how testing was done.

[kaixxx]

@VRehnberg What do you think about using App.new_project() to create a new database with the most recent structure instead of creating all the tables manually? Again, this would make it easier to keep everything up to date.

[VRehnberg]

What do you think about using App.new_project() to create a new database with the most recent structure instead of creating all the tables manually?

@kaixxx I haven't really looked that much at the code so can't answer on particularts. But roughly it depends on the purpose of the tests.

• If you see it as an integration test then use the methods in combination as close to as intended as possible.
• If you see it as a unit test then you need two independent ways of generating the output.

So to take test_load_config_ini as an example:

    def test_load_config_ini(self):
        """ Tests that all config.ini fields are present """

        result, ai_models = App._load_config_ini(self)
        keys = result.keys()
        self.assertEqual(keys, CONFIG_INI_AI_EX4C0559.keys())

this looks like a unit test. You want to check App._load_config_ini in particular then the CONFIG_INI keys shouldn't come from the App._load_config_ini.

So in short, if App.new_project() uses App._load_config_ini() to generate the table, then the test wouldn't really test that much. If it creates the tables independently then it sounds like a good idea.

However, the config.ini in fixtures can probably be replaced by getting self.settings from whatever creates the default values of the config file (I didn't find this so I was sloppy). That said, there is some value in having some config files to run tests on, but these should be from specific releases to the regression testing/testing backwards compatibility.