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

Reduce number of jobs in the Travis CI piwik/piwik build #6413

Closed
mattab opened this issue Oct 10, 2014 · 17 comments
Closed

Reduce number of jobs in the Travis CI piwik/piwik build #6413

mattab opened this issue Oct 10, 2014 · 17 comments
Labels
Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical.
Milestone
Backlog (Help wan...

Comments

@mattab
Copy link
Member

mattab commented Oct 10, 2014

Currently each commit on piwik/piwik repository will trigger a new build on travis CI with 20 different jobs. The goal of this issue is to go down to hopefully 10 jobs instead!

Context

  • Our build configuration has 20 jobs which is rather extreme. we run tests for Unit+Integration+System tests for PHP 5.3, 5.4, 5.5 and 5.6 plus an extra run for Code coverage Setup Coveralls for Piwik Code coverage report #4302 and an extra one for MYSQLI tests (default uses PDO).
    • Running suites in separate jobs has advantage to get feedback about which test suite failed.
    • but it means we create for each commit 3 test suites * 6 = 18 jobs

Proposed solution
Instead we could adopt a smarter strategy... 🎱 which consists of:

  • First we run a job for each test suite in PHP 5.3 in order: one job for system, one job for Core and one for Integration -> it gives devs quick useful feedback.
  • Then we run one job for each PHP version. In this job we run Unit tests, then Integration tests, then System tests. We save two jobs per PHP version.
    • This would save ( 5.4 + 5.5 + 5.6 ) * 2 jobs = 6 jobs
  • We could run MYSQLI tests in one job -> saves 2 jobs
  • maybe we also could run Code coverage in one job -> save 2 jobs

As a result we should get much faster build time and build feedback #6412
@mattab mattab added Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. wontfix If you can reproduce this issue, please reopen the issue or create a new one describing it. Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. labels Oct 10, 2014
@mattab mattab added this to the Short term milestone Oct 10, 2014
@mattab mattab mentioned this issue Oct 10, 2014
Closed
4 tasks
@mattab mattab removed the wontfix If you can reproduce this issue, please reopen the issue or create a new one describing it. label Oct 10, 2014
@sgiehl
Copy link
Member

sgiehl commented Oct 10, 2014

As long as the tests are taking so long to run, I don't see any option to combine the codecoverage builds. Tried that while implementing, but always reached the travis job time limit, and the job was canceled.

tsteur added a commit that referenced this issue Oct 16, 2014
@tsteur
refs #6413 always return a proper return code
2b385c6
tsteur added a commit that referenced this issue Oct 16, 2014
@tsteur
refs #6413 reduce number of jobs in travis.
91caf84 
Not sure if this would work. We would basically only start a job for
1 x AngualarJS
1 x JavaScript
1 x All Tests with PHP 5.4 and MySQLi
3 x All tests with PDO (5.3.3, 5.5 and 5.6)
1 x Unit test with PHP 5.4 and PDO
1 x Integration test with PHP 5.4 and PDO
1 x System test with PHP 5.4 and PDO
= 9 jobs compared to about 18 before
@tsteur
Copy link
Member

tsteur commented Oct 16, 2014

I shrinked it down to 9 jobs + 3 for coverage so far. Not sure if it works yet.

Re coverage: I think it makes sense to remove at least the SystemTests code coverage job as the coverage for this is basically useless. Even for integration tests it is kinda useless but as the job doesn't take that long it is a nice to have. Maybe we can remove the one for SystemTests (which takes > 50min) and run only Unit and Integration instead?

@mnapoli
Copy link
Contributor

mnapoli commented Oct 16, 2014

👍 for removing coverage for system tests.

And even 👍 for removing it for integration tests but that's less critical.

@mnapoli
Copy link
Contributor

mnapoli commented Oct 16, 2014

Anyway the coverage for system tests is killed by Travis because it takes more than 50 minutes: https://travis-ci.org/piwik/piwik/jobs/38106958

So I think we can safely remove it.

tsteur added a commit that referenced this issue Oct 16, 2014
@tsteur
refs #6413 fix script error
4cc1ea9
tsteur added a commit that referenced this issue Oct 16, 2014
@tsteur
refs #6413 do we actually need the config param? don't think so
d8171fd
tsteur added a commit that referenced this issue Oct 16, 2014
@tsteur
refs #6413 remove code coverage generation for system tests.
bbb3fce 
At least for now as mentioned in ticket it does not really make sense
to run them and they take a long time > 50 minutes.
tsteur added a commit that referenced this issue Oct 16, 2014
@tsteur
refs #6413 execute the single test suites before AllTests so we get a…
32f275c 
… faster result for them
@tsteur
Copy link
Member

tsteur commented Oct 16, 2014

FYI: Not running PHP 5.3 tests first as many tests are skipped for PHP 5.3 (which we should not do BTW but that's another topic). Therefore running 5.4 first and the suites separately for 5.4

tsteur added a commit that referenced this issue Oct 16, 2014
@tsteur
refs #6413 need to adjust travis template otherwise tests wont run
7aee5c5
tsteur added a commit that referenced this issue Oct 16, 2014
@tsteur
refs #6413 and again... we need to run PHP 5.4 first
eb09b1b
@diosmosis
Copy link
Member

which we should not do BTW but that's another topic

There are A LOT of skipped tests. Not just for 5.3, I wanted to re-enable them (created a branch for it), but couldn't find the time to finish it.

@tsteur
Copy link
Member

tsteur commented Oct 16, 2014

@sgiehl now that we run only coverage for Unit and Integration tests can we maybe combine it in one job but still get two different code coverages for them?

@diosmosis nice! yeah we should really fix the remaining ones ASAP and it should be no longer done at all in any future test case

@tsteur tsteur mentioned this issue Oct 16, 2014
Merged
@sgiehl
Copy link
Member

sgiehl commented Oct 16, 2014

@tsteur I don't think that would work as coveralls would combine the both results before sending them.

tsteur added a commit that referenced this issue Oct 16, 2014
@tsteur
refs #6413 execute js/angular tests earlier as they are quick to exec…
7480d35 
…ute. Otherwise we have to wait forever for a result
@tsteur
Copy link
Member

tsteur commented Oct 16, 2014

alright. I'm not so deep in this topic therefore maybe stupid question: Can't we create the coverage with the 5.4 Unit and Integration test maybe? So that we can get rid of both completely. I reckon it is not done this way as they take much longer with coverage?

@sgiehl
Copy link
Member

sgiehl commented Oct 16, 2014

Sure we could. I separated them as, as you supposed, they take much longer. And failures caused by code coverage, such as timeouts, are ignored that way.

@tsteur
Copy link
Member

tsteur commented Oct 16, 2014

Just had a look if we can speed up coverage generation but haven't found anything useful. So maybe we leave those two jobs there for now as 20 minutes for integration tests take too long.

While looking for this I noticed disabling xdebug reduces the test time on my instance by about 30% (just normal that it will be faster). For the default tests we won't need xdebug so will check if this is doable. I saw the performance improvement after disabling the extension, not after calling xdebug_disable().

tsteur added a commit that referenced this issue Oct 16, 2014
@tsteur
refs #6413 wondering if this works and gives us faster test performance
1045d59
tsteur added a commit that referenced this issue Oct 16, 2014
@tsteur
refs #6413 show a message if xdebug is loaded but not needed
13e23de
@tsteur
Copy link
Member

tsteur commented Oct 17, 2014

Tests are now about 10-30% faster I think as xdebug is disabled. AllTests from > 22 minutes down to 15minutes. UnitTests from 30 seconds to 23, IntegrationTests from 6:30 to 4:30 or so. Of course travis speed always varies but I can see this on my local instance as well. Should be at least 10% faster. See https://travis-ci.org/piwik/piwik/builds/38222406

@mnapoli
Copy link
Contributor

mnapoli commented Oct 19, 2014

Merged #6459: we now have 9 jobs per build instead of 11. Tests run only on 5.3.3 and 5.6.

@tsteur
Copy link
Member

tsteur commented Oct 19, 2014

Perfect!

@tsteur tsteur closed this as completed Oct 19, 2014
@mattab
Copy link
Member Author

mattab commented Oct 20, 2014

A so very welcome change! 👍

@tsteur
Copy link
Member

tsteur commented Oct 31, 2014

I know it is already closed but don't want to create a new issue for a random thought. Maybe we could remove one more job PHP 5.6 TEST_SUITE=UnitTests MYSQL_ADAPTER=PDO_MYSQL and move the PHP 5.6 Unit Test with Coverage upwards where the previous Unit Test was executed before and do not allow failures for this one. They both test the same.

I think the normal Unit Test takes 2 minutes on Travis whereas the other one with coverage takes about 5 minutes which is not a big difference for me. Unit Test should not fail so often and anyone who needs a fast test result can execute them locally within 10-30 seconds.

@mnapoli
Copy link
Contributor

mnapoli commented Oct 31, 2014

@tsteur Code coverage builds are in the process of being moved to Scrutinizer, so they should disappear from Travis anyway (that'll be 2 less builds): #6556

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical.
Projects
None yet
Milestone
Backlog (Help wanted)
Development

No branches or pull requests
5 participants
@diosmosis @tsteur @mattab @mnapoli @sgiehl