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

Faster bootstrap for PHPUnit tests #6910

Merged
merged 4 commits into from Jan 5, 2015
Merged

Faster bootstrap for PHPUnit tests #6910

merged 4 commits into from Jan 5, 2015

Conversation

mnapoli
Copy link
Contributor

@mnapoli mnapoli commented Dec 30, 2014

When running PHPUnit tests other than unit tests, the bootstrap checks that Piwik server is running correctly.

Instead of hitting the home page (/ 302 that redirect to /index.php? CoreHome…) which takes a couple seconds for the HTTP response, this uses a simpler and faster controller action.

The TestsRunner plugin is disabled in production right? So that new controller shouldn't be a problem.

mnapoli
mnapoli reviewed Dec 30, 2014
View reviewed changes
plugins/TestRunner/plugin.json
"test",
"runner"
],
"homepage": "http:\/\/piwik.org"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

for the record this file was updated by the console

@mnapoli mnapoli added the c: Tests & QA For issues related to automated tests or making it easier to QA & test issues. label Dec 30, 2014
@mnapoli mnapoli self-assigned this Dec 30, 2014
@mnapoli
Copy link
Contributor Author

mnapoli commented Dec 30, 2014

Tests are failing on Travis with this:

Piwik should be running at http://localhost/ but this URL returned an unexpected response: 'The Piwik configuration file couldn't be found and you are trying to access a Piwik page.
» You can install Piwik now
If you installed Piwik before and have some tables in your DB, don't worry, you can reuse the same tables and keep your existing data!

'

Should I use this URL /tests/PHPUnit/proxy/index.php?module=TestRunner&action=check instead of /index.php?module=TestRunner&action=check ?

@mattab
Copy link
Member

mattab commented Dec 30, 2014

Should I use this URL /tests/PHPUnit/proxy/index.php?module=TestRunner&action=check instead of /index.php?module=TestRunner&action=check ?

yes indeed, this URL will use the test DB credentials so it will use the right database

@mnapoli
Copy link
Contributor Author

mnapoli commented Dec 30, 2014

@mattab thanks, done.

@mnapoli
Copy link
Contributor Author

mnapoli commented Dec 30, 2014

Actually it seems it doesn't work, I get the following error on Travis:

SQLSTATE[42S02]: Base table or view not found: 1146 Table piwik_tests.option doesn't exist

Which makes sense.

The version on master hits the home page (/, not the tests index.php), I just wonder why the tests were failing…

@mattab
Copy link
Member

mattab commented Jan 5, 2015

@mnapoli maybe you need some help to troubleshoot this?

@mattab mattab added this to the Piwik 2.11.0 milestone Jan 5, 2015
@mnapoli
Copy link
Contributor Author

mnapoli commented Jan 5, 2015

@mattab I'm trying something.

For the record with a very simple test case I'm down from 2.4 s to 1.4s, which is actually very appreciable when running tests very often (refactoring).

@mnapoli
Copy link
Contributor Author

mnapoli commented Jan 5, 2015

OK to be merged!

mattab pushed a commit that referenced this pull request Jan 5, 2015
Merge pull request #6910 from piwik/faster-tests-bootstrap
7594118 
Faster bootstrap for PHPUnit tests
@mattab mattab merged commit 7594118 into master Jan 5, 2015
@mnapoli mnapoli deleted the faster-tests-bootstrap branch January 5, 2015 01:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@mnapoli mnapoli

Labels
c: Tests & QA For issues related to automated tests or making it easier to QA & test issues.
Projects
None yet
Milestone
Piwik 2.11.0
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@mnapoli @mattab