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

Remove tables_prefix on db tests #15387

Merged
merged 1 commit into from Jul 2, 2020
Merged

Remove tables_prefix on db tests #15387

merged 1 commit into from Jul 2, 2020

Conversation

brunowego
Copy link
Contributor

Remove value of attribute tables_prefix in database_tests to prevent confusion when running tests.

@sgiehl
Copy link
Member

sgiehl commented Jan 13, 2020

what kind of confusion did you have with the prefix? 🤔

@brunowego
Copy link
Contributor Author

@sgiehl thank you for your time. To try explain my point of vision, take a look below. First I create the db test:

mysql \
  -h 127.0.0.1 \
  -P 3306 \
  -u root \
  -p'root' \
  -v <<-\EOSQL
CREATE DATABASE IF NOT EXISTS `matomo_test`;
GRANT ALL PRIVILEGES ON `matomo_test`.* TO 'matomo'@'%';
FLUSH PRIVILEGES;
EOSQL

Second, I import data from db dev to my db test:

mysqldump \
  -h 127.0.0.1 \
  -P 3306 \
  -u matomo \
  -p'matomo' \
  matomo_dev | \
    sed 's/matomodev_/matomotests_/' | \
      mysql \
        -h 127.0.0.1 \
        -P 3306 \
        -u matomo \
        -p'matomo' \
        matomo_test

After that, need necessary configuration to db test:

./console development:enable

./console config:set \
  'database_tests.host="127.0.0.1"' \
  'database_tests.username="matomo"' \
  'database_tests.password="matomo"' \
  'database_tests.dbname="matomo_test"' \
  'database_tests.tables_prefix="matomotests_"'

./console config:set \
  'tests.http_host="127.0.0.1"' \
  'tests.request_uri="/"' \
  'tests.port="8080"'

When run the unit test, I will get failures because is not empty the db prefix table:

./console tests:run \
  --options='--stop-on-error' \
  --testsuite unit
[...]
15) Piwik\Tests\Unit\DataAccess\JoinGeneratorTest::test_generate_getJoinString_allTables
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'log_action AS log_action LEFT JOIN log_link_visit_action AS log_link_visit_action ON log_link_visit_action.idaction_url = log_action.idaction LEFT JOIN log_visit AS log_visit ON log_visit.idvisit = log_link_visit_action.idvisit LEFT JOIN log_conversion AS log_conversion ON log_conversion.idvisit = log_link_visit_action.idvisit LEFT JOIN log_conversion_item AS log_conversion_item ON log_conversion_item.idvisit = log_link_visit_action.idvisit AND `log_conversion_item`.deleted = 0'
+'matomotests_log_action AS log_action LEFT JOIN matomotests_log_link_visit_action AS log_link_visit_action ON log_link_visit_action.idaction_url = log_action.idaction LEFT JOIN matomotests_log_visit AS log_visit ON log_visit.idvisit = log_link_visit_action.idvisit LEFT JOIN matomotests_log_conversion AS log_conversion ON log_conversion.idvisit = log_link_visit_action.idvisit LEFT JOIN matomotests_log_conversion_item AS log_conversion_item ON log_conversion_item.idvisit = log_link_visit_action.idvisit AND `log_conversion_item`.deleted = 0'

/Users/brunowego/Workspace/github.com/matomo-org/matomo/tests/PHPUnit/Unit/DataAccess/LogQueryBuilder/JoinGeneratorTest.php:265

FAILURES!
Tests: 7007, Assertions: 11202, Failures: 15.

@brunowego
Copy link
Contributor Author

Another point, is a exception when tables_prefix is set up in database_tests:

gtime -v php \
  -d memory_limit=8G \
  ./console tests:run-ui --plugin Insights
[Exception]
  To generate OmniFixture for the UI tests, you must set an empty tables_prefix in [database_tests]

@tsteur
Copy link
Member

tsteur commented Jul 1, 2020

Makes sense I reckon. I have actually mine set to empty tables prefix as well. I'll restart the tests and check that it won't break tests and then merge into 4.x

@tsteur tsteur merged commit 50c8303 into matomo-org:3.x-dev Jul 2, 2020
@brunowego brunowego deleted the patch-1 branch July 2, 2020 11:42
jonasgrilleres pushed a commit to 1024pix/pix-analytics that referenced this pull request Sep 22, 2020
jbuget pushed a commit to 1024pix/pix-analytics that referenced this pull request Sep 26, 2020
@mattab mattab added this to the Backlog (Help wanted) milestone Sep 29, 2020
@mattab mattab added the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label Sep 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants