-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[database_tests] section no longer generated when executing php tests #17856
Comments
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 |
|
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).
It should update the regular 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 |
@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: matomo/config/environment/test.php Line 56 in ddfa42e
for the |
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 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.
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. |
It shouldn't create a new database. We would expect it to configure the same database details as in
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 matomo/tests/PHPUnit/Framework/Fixture.php Lines 251 to 255 in ddfa42e
|
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:
database_tests
section is not configured.config/config.ini.php
or is this maybe now expected?
The text was updated successfully, but these errors were encountered: