Either on each test run or only if it doesn't exist yet. To make this useful we should replace
REQUEST_URI automatically. I think we have at least the URI in
SettingsPiwik? Not sure about the rest. If not, maybe we could read them from config?
Currently, a developer has to prepare the DB test config and adjust the phpunit.xml. Having everything only in one place makes it easier to configure and to explain in documentations / blog posts.
By copying the XML all the time it also prevents the issue that sometimes the
.dist files changes and people are not aware that they have to update their local file. I'm not sure if an updated XML file would take effect immediately or with the next test run but either way it would be still better.
Not sure what the best way is to resolve it but would be nice to have it. Maybe we can even get rid of the
.dist file at all and define those 3 variables in bootstrap.php?
If anyone has an idea let me know. I'd like to implement it before 2.9.0 so we can directly explain this in a blog post about testing that I am currently writing.
If we override
phpunit.xml then there is no point in having it anymore. If we don't override it, then it will be copied once, get outdated and it will create problems later that the developer might not understand.
Why not have the tests work by default? I.e. resolve
REMOTE_ADDR & co when running the tests if they are not configured?
REQUEST_URI could be assumed equal to
/ by default since that's how it's setup on most dev machines I guess.
I think also we should consider having a standard dev environment setup and sticking to it. E.g. I'd say let's use PHP's built web-server (because that's the easiest setup ever, that's also the only one documented in the Getting Started guide), and assume everybody does the same. If someone doesn't, then they just customize
phpunit.xml. That way
HTTP_HOST can be assumed equal to
:+1: it would be nice if it works by default eg. for php webserver and otherwise explain how to override those 3 values eg, in a new config.php file
FYI: I tried to copy dist files to phpunit.xml all the time but it was actually only applied on the next test run. Tried quite a few things but nothing really worked so trying to get rid of the dist file at all
Why not keep the dist file? (along with all the other changes that you are doing)
I like being able to customize test suites since right now the unit tests will also run unit tests in plugins, which breaks (since many plugins have failing unit tests).
if you like to customize it, maybe it's worth adding a new testsuite and share it with all? We can of course copy it all the time but wanted to keep it simple
or actually it would be better to fix the tests ^^
and I think it is already possible to say eg "run only unit tests from core" eg
./console tests:run --testsuite unit tests/PHPUnit
Why is there a need to copy it? phpunit will by default use the phpunit.xml.dist file if no phpunit.xml exists.
+1 for keeping phpunit.xml.dist to be able to customize it (even if it should not be required for most devs)
phpunit.xml.dist is standard practice and supported by PHPUnit. It is meant for exactly this: developers can customize the config locally.
phpunit is run, it will look for a
phpunit.xml file, then for a dist file if it can't find one. So I don't see any reason not to do it. There is no need to copy the file.
ok I didn't know this is supported by PHPUnit so makes sense. As mentioned ideally the test runner is flexible and we make use of groups and testsuites so it is not needed to adjust it as we don't want other devs having to adjust it.
[database_tests] is not configured yet shall we simply copy the
[database] password to this section hoping they use a user having the permission to create databases etc?
@diosmosis I am not sure whether our tests are supposed to use an empty
tables_prefix? I think there was something, at least for UI tests. If so, I could make sure that it is empty in
OK. So should we maybe change the default to an empty string here https://github.com/piwik/piwik/blob/2.9.0-b9/config/global.ini.php#L35 ? This would make sure by default tests are executed the same way as on travis. On the other side if someone configures to use the same database name for Piwik and tests we might delete the actual installation when
tables_prefix is empty in
database_tests. This should not happen as
piwik_ by default and if so it would be only a development installation... Not sure what to do.
Fixture.php has a check where if fails to drop the database if the requested database name is the same as in [database], so currently it shouldn't be allowed to use the same database in tests.
Hi @tsteur closing as it looks fixed. if not fully fixed maybe create a new issue for follow up.
This is not done yet see comments waiting for your input re database configuration. Would prefer to leave this open as part of the discussion about this was already here.
What's the next step here @tsteur ? maybe it would be better in
Short term if it is low effort and useful?
It is minimal effort and Piwik 2.12.0 would be fine