@peterhashair opened this Pull Request on October 14th 2021 Contributor

Description:

Fixes: #17856
update database test work without section set. use getenv('TEST_SUITE') to diff the travis and local.

Review

@peterhashair commented on October 18th 2021 Contributor

@sgiehl Sorry for the confusion. The purpose of this change is to run Matomo PHP tests locally without setup [database_tests] section in the config.ini.php. So it should copy the username , password , host from [database] then make a matomo_tests database.
https://github.com/matomo-org/matomo/blob/852c3bdfa59c427d40c50286afe5e532e56736b8/tests/PHPUnit/bootstrap.php#L72-L75

I think those three lines init an empty test environment, the forceSave won't append default database connection to [database_tests] in the config.ini.php file, the local PHP tests doing database access deny . But if I removed it, Travis start to fail, so I add getenv to different 2 environments.

https://github.com/matomo-org/matomo/blob/852c3bdfa59c427d40c50286afe5e532e56736b8/tests/PHPUnit/Framework/Fixture.php#L265
Remove this line. Because after the test runs locally, the code alters the [database] section. A forceSave() has been called.
https://github.com/matomo-org/matomo/blob/249adc21db61899ae1d908a17da4ada46e663b88/plugins/Installation/tests/resources/config.ini.php#L12
Add this host = "127.0.0.1", purpose to pass Plugins\CoreAdminHome\tests\Integration\Commands\ConfigGetTesttests/PHPUnit/proxy/index.phpassert database access deny. Because I empty the[database_tests]localhost inglobal.ini.phpto auto pass from[database]`.

@sgiehl commented on October 19th 2021 Member

@peterhashair The problem with not using the TestingEnvironmentManipulator locally at all will cause tests to overwrite your local config. Similar to the database section update (you removed it), now any test can manipulate it. Try e.g. running the test MultipleSitesArchivingTest. This will then change your config file:
https://github.com/matomo-org/matomo/blob/6d744db7f3f7a55fc4bb1ca573e937f0832c642b/tests/PHPUnit/System/MultipleSitesArchivingTest.php#L47-L48

@peterhashair commented on October 19th 2021 Contributor

@sgiehl that actually make sense, update it a little bit. so init again after the config database_tests is set.

@peterhashair commented on October 27th 2021 Contributor

@sgiehl that one good to go?

This Pull Request was closed on October 28th 2021
Powered by GitHub Issue Mirror