Skip to content
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

Jump to bottom

update database test work without section set #18151

Merged
merged 14 commits into from Oct 28, 2021
Merged

update database test work without section set #18151

merged 14 commits into from Oct 28, 2021

Conversation

peterhashair
Copy link
Contributor

@peterhashair peterhashair commented Oct 14, 2021 •
edited

Description:

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

Review

Peter Zhang added 11 commits October 14, 2021 16:41
update database test
3ac6a31 
update database test
update tests
5ec8c96 
update tests
Merge branch '4.x-dev' into m-18144-database-test-fix
6002865
add matomo_tests to exception
c0bb4c1 
add matomo_tests to exception
Merge branch '4.x-dev' into m-18144-database-test-fix
5d8b345
test env on travis
ce5664f 
test env on travis
test database change on travis
f5020d6 
test database change on travis
update tests
b156d29 
update tests
update test
4ca8082 
update test
update test database host
25a94bd 
update test database host
update tests
2135ad2 
update tests
@peterhashair peterhashair marked this pull request as ready for review October 18, 2021 03:15
@peterhashair peterhashair added Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. labels Oct 18, 2021
@peterhashair peterhashair added this to the 4.6.0 milestone Oct 18, 2021
@peterhashair
Copy link
Contributor Author

peterhashair commented Oct 18, 2021 •
edited

@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.

matomo/tests/PHPUnit/bootstrap.php

Lines 72 to 75 in 852c3bd

$vars = new TestingEnvironmentVariables();
$vars->delete();
Environment::setGlobalEnvironmentManipulator(new TestingEnvironmentManipulator($vars));

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.

matomo/tests/PHPUnit/Framework/Fixture.php

Line 265 in 852c3bd

self::getConfig()->database['dbname'] = $this->dbName;

Remove this line. Because after the test runs locally, the code alters the [database] section. A forceSave() has been called.

matomo/plugins/Installation/tests/resources/config.ini.php

Line 12 in 249adc2

host = "127.0.0.1"

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 in global.ini.phpto auto pass from[database]`.

@sgiehl
Copy link
Member

sgiehl commented Oct 19, 2021

@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:

matomo/tests/PHPUnit/System/MultipleSitesArchivingTest.php

Lines 47 to 48 in 6d744db

Config::getInstance()->General['enable_processing_unique_visitors_multiple_sites'] = 1;
Config::getInstance()->Tracker['enable_fingerprinting_across_websites'] = 1;

Update bootstrap.php
d1adba6 
update bootstrap to recreate empty test
@peterhashair
Copy link
Contributor Author

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

@peterhashair
Copy link
Contributor Author

@sgiehl that one good to go?

sgiehl
sgiehl approved these changes Oct 28, 2021
View reviewed changes
Copy link
Member

@sgiehl sgiehl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checked that again and it seems to work as expected now.

@sgiehl sgiehl merged commit 7de92b4 into 4.x-dev Oct 28, 2021
@sgiehl sgiehl deleted the m-18144-database-test-fix branch October 28, 2021 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sgiehl sgiehl sgiehl approved these changes

Assignees
No one assigned
Labels
Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Milestone
4.6.0
Development

Successfully merging this pull request may close these issues.

[database_tests] section no longer generated when executing php tests
2 participants
@peterhashair @sgiehl