[database_tests] section no longer generated when executing php tests #17856

to reproduce remove the section [database_tests] from your local tests. It will then result in a database access exception.

That's because in https://github.com/matomo-org/matomo/blob/4.4.1/tests/PHPUnit/bootstrap.php#L126 it uses a mock config which has "allowSave = false".

It must have regressed at some point but not sure where.

The goals are:

• [ ] Not throw an exception when database_tests section is not configured.
• [ ] Automatically pre-fill the database_tests section in the config/config.ini.php

or is this maybe now expected?

Not sure what the expected behavior should be. When automatically storing the config it might have the risk that it's the same as the normal db config. There is no need to define a table prefix when installing Matomo, so tests could overwrite the normal database.

We could maybe have some new command like .\console tests:setup, that ask for various parameters and adjusts the config accordingly. For UI tests there might also http_host and request_uri be needed.

./console tests:setup seems like a good idea. Could it automatically be triggered when the config is missing and ask:

Do you want to configure a test DB now? (Y/n)

I'm not sure what the point of a command is which in the end then also just does it maybe "automatically". Then this logic could be executed directly when bootstrapping.

We could check that the test prefix is different to the regular database (which it is in > 80% of the cases since "matomo_" prefix is the default so it will work most of the time).

• If the empty test prefix is different to the regular DB prefix, then we can configure the tests section automatically.
• If test section is not configured and the regular DB prefix is empty, we could throw an exception when running the tests and let the user know to configure the test DB manually. That would then likely mean the user needs to configure a different DB name for the tests as it means the user already is using an empty prefix and the tests need to run with an empty DB prefix or the user could reinstall Matomo with a DB prefix by deleting the config file but then all data gets lost.

It should update the regular config/config.ini.php when no test DB section is configured, which it used to do AFAIK.

Basically, the goal is that the user does not need to configure this section.

The logic for the UI tests should also configure additionally the config/config.ini.php if that's not the case yet https://github.com/matomo-org/matomo/blob/4.5.0-b1/tests/PHPUnit/bootstrap.php#L85-L94

@tsteur, i agree. It should check the prefixes are the same as the regular DB parameter and iff they are the same throw an error. Otherwise it should just go ahead and save that config.

Should this mean that we put the logic here:

https://github.com/matomo-org/matomo/blob/4.5.0-b1/tests/PHPUnit/bootstrap.php#L85-L94

and pass in true here:

https://github.com/matomo-org/matomo/blob/ddfa42e16f6648a1f54d2912fcc897e0ae07b75e/config/environment/test.php#L56

for the $allowSave parameter?

Hard to say, I'm not too much into this topic. I only know this used to work but must have regressed at some point. The goal will be that it configures this automatically saves the regular config/config.ini.php if it's not configured yet. If the section is already configured, then we don't want to change anything. Not sure if allowSave is false for a reason maybe. And we need to make sure if we save there, that it doesn't overwrite any other setting in config/config.ini.php. It should only overwrite this particular setting there. If the test bootstrap process sets maybe other configs, these should not be written.

Might be easier to create a new config instance there just in case something else would change the config by accident. I haven't looked into it too much though.

@tsteur Just confirm the logic here.

• when the [database_tests] is not set, it will create a new database with the _test suffix through the same connection that already exists in the [database] section

• If the connection failed because of a password (like username not set on %) or [datababse] dbname is the same as [database_test] about to generate, we throw an error message, require the user to do a manual config. Otherwise, just continue PHP tests.

It's that sounds about right?

Another approach is use sqlite driver, store a sqlite file into /tmp/matomo_test.sqlite. If not specify setup, always use sqlite.

when the [database_tests] is not set, it will create a new database with the _test suffix through the same connection that already exists in the [database] section

It shouldn't create a new database. We would expect it to configure the same database details as in [database] with the only difference that the prefix will be set to an empty string. Unless in [database] the table prefix is already an empty string, in that case we would probably throw an exception and get users to manually configure a different [database_tests].

If the connection failed because of a password (like username not set on %) or [datababse] dbname is the same as [database_test] about to generate, we throw an error message, require the user to do a manual config. Otherwise, just continue PHP tests.

Yes, in that case we would throw an exception 👍

Our queries actually aren't compatible with sqlite unfortunately.

@tsteur not sure I am correct when I fix the database_test issue, if I put them into the same database when I run PHP tests like ./console tests:setup-fixture OmniFixture or any system tests. it seems like there is a function called trying to drop the entire database.
https://github.com/matomo-org/matomo/blob/ddfa42e16f6648a1f54d2912fcc897e0ae07b75e/tests/PHPUnit/Framework/Fixture.php#L251-L255

It'll be the same database but the tables_prefix should be different. AFAIK by default Matomo uses a table prefix matomo_ meaning if you set an empty prefix in the tests, they shouldn't overwrite each other. However, if the user has configured no table prefix (meaning an empty table prefix) in [database] then we can't configure this automatically and we would throw an exception.

I checked again though and by default we indeed use matomo_tests database. Meaning all we need to configure there is

[database_tests]
host = "the host from [database]"
username = "the username from [database]"
password = "the password from [database]"

If no [database_tests]username is not configured yet

This is what prepareTestDatabaseConfig already does. The logic is already there. It's just not modifying the correct config.

You can temporarily remove the [database_tests] section from your local config/config.ini.php. Then try to run an integration test and it will fail like this:

Expected outcome would be that the tests run without that error.