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

Upgrade travis to trusty environment #12087

Merged
merged 3 commits into from Oct 13, 2017
Merged

Upgrade travis to trusty environment #12087

merged 3 commits into from Oct 13, 2017

Conversation

mneudert
Copy link
Member

Pull Request to integrate/validate changes made in matomo-org/travis-scripts#36.

The submodule change to the fork should obviously be ignored. The "bump submodules" commit integrates changes from the travis repository after running ./console generate:travis-yml --core and should be replaced before merging with a proper submodule update.

@mattab mattab modified the milestones: 3.1.1, 3.1.2 Sep 21, 2017
@mattab
Copy link
Member

mattab commented Sep 21, 2017

Hi @mneudert

Excellent progress 👍

I checked all the UI tests and most can be safely overriden, except for one issue.

Most UI tests failures are due to:

  • small changes in the fonts (especially for text in italic and unicode text),
  • upgraded Mysql version (the Mysql version is displayed in the 'System summary' widget in the dashboard)
  • the web server now runs on port 3000 (seen here)
  • missing GD /freetype

Blocking issue: missing GD freetype

The missing GD/freetype issue (visible in some of the ui tests such as this System check test) is the only blocking issue because we are testing that our static graphs generate correctly in the following tests:

-> is it possible to install the GD/freetype in Travis so these screenshot tests will execute as expected?

@mattab mattab added the c: Tests & QA For issues related to automated tests or making it easier to QA & test issues. label Sep 21, 2017
@mattab
Copy link
Member

mattab commented Sep 21, 2017

@mneudert tried to trigger a few builds but it failed due to wrong submodule. Don't really understand why your last commit triggered build OK while mine didn't 😕 Looking forward to merging this hopefully 👍

@mneudert
Copy link
Member Author

Hmpf, yeah. While testing some submodules (LoginLdap war completely unimpressed about the changes!) I found a whitespace error in a script and have missed to update the reference here. The submodule commit is gone due to a force push, nothing you did wrong ^^ I have cleaned that mess up and you should now properly be able to trigger builds.

For the GD changes I am searching for a solution. It would be quite unexpected to not be able to add the missing parts. Otherwise I would expect more users to mention the graphs are broken/not visible because of the same setup problems.

@mattab
Copy link
Member

mattab commented Sep 21, 2017

@mneudert maybe we could contact Travis CI and ask them for help? They usually reply quickly and are helpful so we shouldn't hesitate in case they maybe know a solution or a tip.

@mneudert
Copy link
Member Author

If we switch the UITests to php-5.6 the screenshots in question seem to generate just fine and gd_info() announces the existence of freetype.

So we could upgrade the PHP version or ask travis to change the installed PHP version to include freetype support. Or perhaps both :)

Added a php-version-flip-commit to see how a full test run holds up to my observations...

@mneudert
Copy link
Member Author

mneudert commented Sep 25, 2017

Not sure anymore whether lowering any of the test requirements to php-5.5 is a good idea as there seem to be some requirements hidden in the tests:

2) Piwik\Tests\System\EcommerceOrderWithItemsTest::testImagesIncludedInTests
(This should not occur on Travis CI server as we expect these tests to run there).
Scheduled reports generated during integration tests will not contain the image graphs.
For tests to generate images, use a machine with the following specifications :
    OS = linux, PHP = 5.6 and GD = 2.1.0

@mattab
Copy link
Member

mattab commented Sep 25, 2017

@mneudert fyi you can try change the requirements of the tests, so they run on Travis on 5.5. The idea behind the message is to make sure they at least run there. The reason they are restricted to one PHP version and GD is because GD has some minor pixel differences depending on the php/gd versions and we need to always run them on the same one (but we can change the versions and updated the images)

@mattab
Copy link
Member

mattab commented Sep 25, 2017

Also there are a few randomly failing tests:

4 failures here



There were 4 failures:

1) Piwik\Plugins\CustomDimensions\tests\System\AutoSuggestTest::testApi with data set #1 (array('API.getSuggestedValuesForSegment'), array(1, '2017-08-21 00:00:00', array('year'), array('dimension3', 1), '_actionScope'))
Piwik\Plugins\CustomDimensions\tests\System\AutoSuggestTest: Differences with expected in '/home/travis/build/piwik/piwik/plugins/CustomDimensions/tests/System/processed/test__actionScope__API.getSuggestedValuesForSegment.xml'
Failed asserting that two DOM documents are equal.
--- Expected
+++ Actual
@@ @@
 <?xml version="1.0"?>
 <result>
   <row>en</row>
-  <row>value3</row>
   <row>value5 3</row>
   <row/>
+  <row>value3</row>
 </result>

/home/travis/build/piwik/piwik/tests/PHPUnit/Framework/TestRequest/Response.php:76
/home/travis/build/piwik/piwik/tests/PHPUnit/Framework/TestCase/SystemTestCase.php:375
/home/travis/build/piwik/piwik/tests/PHPUnit/Framework/TestCase/SystemTestCase.php:515
/home/travis/build/piwik/piwik/plugins/CustomDimensions/tests/System/AutoSuggestTest.php:33

2) Piwik\Plugins\CustomDimensions\tests\System\AutoSuggestTest::test_getMostUsedActionDimensionValues_shouldReturnMostUsedValues
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
     0 => 'en'
-    1 => 'value3'
-    2 => 'value5 3'
-    3 => ''
+    1 => 'value5 3'
+    2 => ''
+    3 => 'value3'
 )

/home/travis/build/piwik/piwik/plugins/CustomDimensions/tests/System/AutoSuggestTest.php:71

3) Piwik\Plugins\CustomDimensions\tests\System\AutoSuggestTest::test_getMostUsedActionDimensionValues_shouldApplyLimit
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
     0 => 'en'
-    1 => 'value3'
+    1 => 'value5 3'
 )

/home/travis/build/piwik/piwik/plugins/CustomDimensions/tests/System/AutoSuggestTest.php:78

4) Piwik\Plugins\CustomDimensions\tests\System\AutoSuggestTest::test_getMostUsedActionDimensionValues_shouldApplyIndex
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
     0 => 'en_US'
-    1 => 'value5'
-    2 => '343'
-    3 => 'value5 5'
+    1 => '343'
+    2 => 'value5 5'
+    3 => 'value5'
 )

/home/travis/build/piwik/piwik/plugins/CustomDimensions/tests/System/AutoSuggestTest.php:92

1 failure here



There was 1 failure:

1) Piwik\Plugins\CustomDimensions\tests\System\AutoSuggestTest::testApi with data set #1 (array('API.getSuggestedValuesForSegment'), array(1, '2017-08-21 00:00:00', array('year'), array('dimension3', 1), '_actionScope'))
Piwik\Plugins\CustomDimensions\tests\System\AutoSuggestTest: Differences with expected in '/home/travis/build/piwik/piwik/plugins/CustomDimensions/tests/System/processed/test__actionScope__API.getSuggestedValuesForSegment.xml'
Failed asserting that two DOM documents are equal.
--- Expected
+++ Actual
@@ @@
 <?xml version="1.0"?>
 <result>
   <row>en</row>
-  <row>value3</row>
   <row>value5 3</row>
   <row/>
+  <row>value3</row>
 </result>

/home/travis/build/piwik/piwik/tests/PHPUnit/Framework/TestRequest/Response.php:76
/home/travis/build/piwik/piwik/tests/PHPUnit/Framework/TestCase/SystemTestCase.php:375
/home/travis/build/piwik/piwik/tests/PHPUnit/Framework/TestCase/SystemTestCase.php:515
/home/travis/build/piwik/piwik/plugins/CustomDimensions/tests/System/AutoSuggestTest.php:33

Solution to randomly failing tests

These issues are caused by a difference in the sorting of elements. I think caused by this PHP sort behavior documented here

If two members compare as equal, their relative order in the sorted array is undefined.

This could maybe be fixed by adding another sort where when elements have the same 'count', we could sort them in alphabetical order to ensure the order is constant and does not depend on the sort implementation?

@mattab
Copy link
Member

mattab commented Sep 25, 2017

So we could upgrade the PHP version

@mneudert sounds good to upgrade the PHP version for UI tests 👍

@mneudert
Copy link
Member Author

I tried modifying the SystemTests to run with PHP 5.5 in a separate branch (build here) but I think the GD library broke it. As it seems there is just not FreeType Support for PHP 5.5 on Travis at the moment, only for PHP 5.6 or the old precise images.

Already reached out to Travis to see if that is intentional or could be resolved: travis-ci/travis-ci#8510


From what I see in your builds all flapping tests are includede in the CustomDimensions plugin, right? That would make it easy to get them fixed as we would be looking at a rather confined space...

@mattab
Copy link
Member

mattab commented Sep 27, 2017

Yes they're all tests in the customDimensions plugin.

@mneudert
Copy link
Member Author

mneudert commented Oct 5, 2017

As it seems the missing FreeType support on PHP 5.5 for the new Travis infrastructure is a given. Not by intention but perhaps also not changed quickly.

For now I switched the UITests to the old infrastructure in order to have a working PHP 5.5 build in the matrix because I really don't like dropping the low end requirement. Using the custom matrix we only have to change 2 lines to change the infrastructure.

@mattab
Copy link
Member

mattab commented Oct 5, 2017

FYI: There's also this failing test in the last build:

There was 1 failure:

1) Piwik\Plugins\SegmentEditor\tests\Integration\SegmentEditorTest::test_UpdateSegment

Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
-    'idsegment' => 2
+    'idsegment' => '2'
     'name' => 'NEW name'
     'definition' => 'searches==0'
     'login' => 'superUserLogin'
-    'enable_all_users' => 0
-    'enable_only_idsite' => 0
-    'auto_archive' => 0
+    'enable_all_users' => '0'
+    'enable_only_idsite' => '0'
+    'auto_archive' => '0'
     'ts_created' => '2017-10-05 18:50:'
-    'ts_last_edit' => '2017-10-05 18:51:'
-    'deleted' => 0
+    'ts_last_edit' => '2017-10-05 18:50:'
+    'deleted' => '0'
 )

@mneudert looking forward to merging this one (which requires https://github.com/piwik/plugin-CustomDimensions/issues/71 )

@mneudert
Copy link
Member Author

mneudert commented Oct 6, 2017

Race conditions ❤️

As far as I remember that error did not occur in any of the previous runs. As the ts_last_edit is auto generated it might have been a simple race condition with the test taking long enough to move over to the next second.

As the update is done after generating the expected result values I can only think of modifying the result to expect not a specific time but a range check for something like "ts_created + 2 seconds". Will try something like that 👍

@mneudert
Copy link
Member Author

mneudert commented Oct 6, 2017

After reading properly I saw we are talking about minutes not seconds. Are there enough tests with such a time handling that we might benefit more from a general logic?

Thinking about "assertTimeWithinSeconds($expected, $result, $seconds)" and removing it from the other result assertions...

@mattab
Copy link
Member

mattab commented Oct 8, 2017

As far as I remember that error did not occur in any of the previous runs.

nice catch. I'd say we can easily ignore it as I don't remember seeing this race condition many times...

@mattab
Copy link
Member

mattab commented Oct 10, 2017

Hi @mneudert would you mind updating the submodule? the build may be green this time and we can merge this useful PR 👍

@mattab mattab modified the milestones: 3.2.0, 3.3.0, 3.2.1 Oct 10, 2017
@mneudert
Copy link
Member Author

Tests look rather good to me now 👍

When merging one should take care to remove the first two commits beforehand. After merging the travis-scripts PR a run of generate:travis-yml should properly update the file to the new state.

@mattab
Copy link
Member

mattab commented Oct 11, 2017

When merging one should take care to remove the first two commits beforehand.

@mneudert I'm not sure about removing commits from PR. Would you mind merging the PR yourself maybe? I've reviewed it and happy with it 👍

@mneudert mneudert closed this Oct 12, 2017
@mneudert mneudert deleted the a-little-less-sudo branch October 12, 2017 22:16
@mneudert mneudert restored the a-little-less-sudo branch October 12, 2017 22:16
@mneudert mneudert reopened this Oct 12, 2017
@mneudert
Copy link
Member Author

Hmpf, hit the wrong button on a different page. So tests need a second run to show their results here...

(previous: https://travis-ci.org/piwik/piwik/builds/287273686)

@mneudert
Copy link
Member Author

Failed screenshot tests look like some usual noise.

UIIntegrationTest_segmented_visitorlog.png failed to upload due to some "Invalid authentication key" but otherwise no diffs generated.

@sgiehl
Copy link
Member

sgiehl commented Oct 13, 2017

I gonna merge this now 🎉

@sgiehl sgiehl merged commit 49c423f into matomo-org:3.x-dev Oct 13, 2017
@mneudert mneudert deleted the a-little-less-sudo branch October 13, 2017 08:37
@matomo-org matomo-org deleted a comment from MatomoForumBot Dec 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: Tests & QA For issues related to automated tests or making it easier to QA & test issues.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants