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

Phpunit speedtrap #12763

Closed
wants to merge 7 commits into from

Conversation

@szepeviktor
Copy link
Contributor

commented Apr 23, 2018

The purpose of this Pr is to measure long-running unit tests.

Was #12760

szepeviktor added 7 commits Apr 23, 2018
@szepeviktor

This comment has been minimized.

Copy link
Contributor Author

commented Apr 23, 2018

Results

SystemTests

You should really fix these slow tests (>500ms)...
 1. 215204ms to run Piwik\Tests\System\ArchiveCronTest:testArchivePhpCron
 2. 59605ms to run Piwik\Tests\System\TwoVisitorsTwoWebsitesDifferentDaysTest:testApi with data set #7
 3. 54062ms to run Piwik\Tests\System\AutoSuggestAPITest:testAnotherApi with data set #92
 4. 51416ms to run Piwik\Tests\System\AutoSuggestAPITest:testAnotherApi with data set #96
 5. 51372ms to run Piwik\Tests\System\AutoSuggestAPITest:testAnotherApi with data set #90
 6. 50215ms to run Piwik\Tests\System\AutoSuggestAPITest:testAnotherApi with data set #74
 7. 48881ms to run Piwik\Tests\System\AutoSuggestAPITest:testAnotherApi with data set #72
 8. 48671ms to run Piwik\Tests\System\AutoSuggestAPITest:testAnotherApi with data set #94
 9. 47777ms to run Piwik\Tests\System\AutoSuggestAPITest:testAnotherApi with data set #66
 10. 47513ms to run Piwik\Tests\System\AutoSuggestAPITest:testAnotherApi with data set #58
...and there are 202 more above your threshold hidden from view
Time: 32.84 minutes, Memory: 368.00MB
@szepeviktor

This comment has been minimized.

Copy link
Contributor Author

commented Apr 23, 2018

Something very fundamental takes ~50 seconds per unit test

@mattab mattab added the Needs Review label Apr 24, 2018

@mattab mattab added this to the 3.6.0 milestone Apr 24, 2018

@mattab mattab modified the milestones: 3.6.0, 3.5.1 May 8, 2018

@mattab mattab added the c: Tests & QA label May 8, 2018

@mattab

This comment has been minimized.

Copy link
Member

commented May 8, 2018

Thank you @szepeviktor for creating your first PR, that's really useful 👍 making our tests faster has become a top priority as the System tests often fail as they exceed 40min.

From this output it looks like possibly Piwik\Tests\System\AutoSuggestAPITest:testAnotherApi should be tackled first as this causes probably 15+min or more in itself. (these tests check that each dimension can be used as segment, and have at least a visit matching in the fixture.)

You should really fix these slow tests (>500ms)...
 1. 215204ms to run Piwik\Tests\System\ArchiveCronTest:testArchivePhpCron
 2. 59605ms to run Piwik\Tests\System\TwoVisitorsTwoWebsitesDifferentDaysTest:testApi with data set #7
 3. 54062ms to run Piwik\Tests\System\AutoSuggestAPITest:testAnotherApi with data set #92
 4. 51416ms to run Piwik\Tests\System\AutoSuggestAPITest:testAnotherApi with data set #96
 5. 51372ms to run Piwik\Tests\System\AutoSuggestAPITest:testAnotherApi with data set #90
 6. 50215ms to run Piwik\Tests\System\AutoSuggestAPITest:testAnotherApi with data set #74
 7. 48881ms to run Piwik\Tests\System\AutoSuggestAPITest:testAnotherApi with data set #72
 8. 48671ms to run Piwik\Tests\System\AutoSuggestAPITest:testAnotherApi with data set #94
 9. 47777ms to run Piwik\Tests\System\AutoSuggestAPITest:testAnotherApi with data set #66
 10. 47513ms to run Piwik\Tests\System\AutoSuggestAPITest:testAnotherApi with data set #58

edit: @szepeviktor would it be maybe possible to display the top 100 slow tests instead of top 10?

@szepeviktor

This comment has been minimized.

Copy link
Contributor Author

commented May 8, 2018

@mattab Do you have a 1 page summary on how to get from cloning matomo repo to running tests?
Maybe a Dockerfile??

@sgiehl

This comment has been minimized.

Copy link
Member

commented May 8, 2018

@szepeviktor

This comment has been minimized.

Copy link
Contributor Author

commented May 8, 2018

I think I need to create a Dockerfile to be able to breath

@szepeviktor

This comment has been minimized.

Copy link
Contributor Author

commented May 8, 2018

Could you help me how piwik_tests db gets populated on Travis?

@szepeviktor

This comment has been minimized.

Copy link
Contributor Author

commented May 8, 2018

Adding comments to the testing system (travis.yml, shell scripts) may help a lot.

@diosmosis

This comment has been minimized.

Copy link
Member

commented May 18, 2018

@szepeviktor I use docker half of the time for development: https://github.com/diosmosis/matomo-docker-dev . It's definitely not official, so it may or may not work for you, I can't promise any real support and I will make changes to it as I need them.

Use docker-compose up to start it up (and all the other containers I use when developing). Install matomo by visiting localhost:3000, and run commands via the console script in the readme, eg, /path/to/matomo-docker-dev/console tests:run ./tests/PHPUnit/System/OneVisitorTwoVisitsTest.php (this will run the console php script in the matomo container).

As for how the test database is populated, it is created, setup & filled by the https://github.com/matomo-org/matomo/blob/3.x-dev/tests/PHPUnit/Framework/Fixture.php class and its descendants. (Fair warning, it sets up the entire matomo test environment, and so is a very large, hard to read class, and may make your eyes bleed.) Some test setup code is located elsewhere like in the https://github.com/matomo-org/matomo/blob/3.x-dev/tests/PHPUnit/Framework/TestingEnvironmentManipulator.php class.

@szepeviktor

This comment has been minimized.

Copy link
Contributor Author

commented May 18, 2018

Thank you.

@diosmosis
Copy link
Member

left a comment

  • The minimum execution time threshold needs to be increased for system tests, as they will always be slower than unit tests. Not sure what the best value is since some will always be long running. Perhaps it would be good to make the speed trap listener optional in some way.
  • There's a conflict in the composer file that needs to be fixed.
sudo: false
- php: 5.5.33
env: TEST_SUITE=UITests MYSQL_ADAPTER=PDO_MYSQL UITEST_EXTRA_OPTIONS="--run-second-half-only"
sudo: false

This comment has been minimized.

Copy link
@diosmosis

diosmosis May 22, 2018

Member

These changes need to be reverted before this PR can be merged.

@szepeviktor

This comment has been minimized.

Copy link
Contributor Author

commented May 22, 2018

Thank you @diosmosis!

This PR is a rather like a debugging session.
It's goal is to find out what takes ~50 second per test.
Up to now I wasn't able to find out.

@diosmosis

This comment has been minimized.

Copy link
Member

commented May 24, 2018

@szepeviktor Gotcha, will remove this from the current milestone then, feel free to create PRs when you find some optimizations :)

@diosmosis diosmosis removed this from the 3.5.1 milestone May 24, 2018

@mattab

This comment has been minimized.

Copy link
Member

commented Jun 26, 2018

Thanks @szepeviktor for creating this PR. I've added a comment in the related issue here: #12691 (comment)
I'll close this PR now, but feel free to re-open it as we could in theory add the speedtrap by default in Matomo.

@mattab mattab closed this Jun 26, 2018

@szepeviktor szepeviktor deleted the szepeviktor:phpunit-speedtrap branch Jun 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.