jenkins-bot has submitted this change and it was merged.

Change subject: Remove defaults for choices in test fixtures
......................................................................


Remove defaults for choices in test fixtures

The use of default values in test fixtures helped make the fixtures
more compact, but as the fixtures become more complex, it will
make them more difficult to read. This is the first step in removing
them. The next step will be to remove them for the setup property
of test fixtures (used by PHPUnit tests). Note that some tweaking on
the PHPUnit side was needed here. That'll be fleshed out in the next
step.

Also added a tidbit of fixture documentation (since comments are not
allowed in json).

Change-Id: I1e1e807deb86806f9c19e42dabeb857529e43e22
---
M tests/ApiCentralNoticeBannerChoiceDataTest.php
M tests/BannerAllocationCalculatorTest.php
M tests/BannerChoiceDataProviderTest.php
M tests/CNBannerChoicesResourceLoaderModuleTest.php
M tests/CentralNoticeTestFixtures.php
M tests/data/AllocationsFixtures.json
A tests/data/allocation_fixtures_doc.md
M 
tests/qunit/ext.centralNotice.bannerController.lib/bannerController.lib.tests.js
8 files changed, 321 insertions(+), 59 deletions(-)

Approvals:
  Awight: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/tests/ApiCentralNoticeBannerChoiceDataTest.php 
b/tests/ApiCentralNoticeBannerChoiceDataTest.php
index b140f1b..3aad3be 100644
--- a/tests/ApiCentralNoticeBannerChoiceDataTest.php
+++ b/tests/ApiCentralNoticeBannerChoiceDataTest.php
@@ -24,6 +24,8 @@
         * @dataProvider 
CentralNoticeTestFixtures::allocationsTestCasesProvision
         */
        public function testBannerChoiceResponse( $name, $testCase ) {
+
+               $this->cnFixtures->prepareTestcase( $testCase );
                $this->cnFixtures->setupTestCase( $testCase['setup'] );
 
                $ret = $this->doApiRequest( array(
diff --git a/tests/BannerAllocationCalculatorTest.php 
b/tests/BannerAllocationCalculatorTest.php
index 448b1b9..101d93c 100644
--- a/tests/BannerAllocationCalculatorTest.php
+++ b/tests/BannerAllocationCalculatorTest.php
@@ -24,6 +24,8 @@
         * @dataProvider 
CentralNoticeTestFixtures::allocationsTestCasesProvision
         */
        public function testAllocations( $name, $testCase ) {
+
+               $this->cnFixtures->prepareTestcase( $testCase );
                $this->cnFixtures->setupTestCase( $testCase['setup'] );
 
                $allocationsProvider = new BannerChoiceDataProvider(
diff --git a/tests/BannerChoiceDataProviderTest.php 
b/tests/BannerChoiceDataProviderTest.php
index fde631f..035a657 100644
--- a/tests/BannerChoiceDataProviderTest.php
+++ b/tests/BannerChoiceDataProviderTest.php
@@ -24,6 +24,8 @@
         * @dataProvider 
CentralNoticeTestFixtures::allocationsTestCasesProvision
         */
        public function testProviderResponse( $name, $testCase ) {
+
+               $this->cnFixtures->prepareTestcase( $testCase );
                $this->cnFixtures->setupTestCase( $testCase['setup'] );
 
                $allocationsProvider = new BannerChoiceDataProvider(
diff --git a/tests/CNBannerChoicesResourceLoaderModuleTest.php 
b/tests/CNBannerChoicesResourceLoaderModuleTest.php
index 285a509..c5be722 100644
--- a/tests/CNBannerChoicesResourceLoaderModuleTest.php
+++ b/tests/CNBannerChoicesResourceLoaderModuleTest.php
@@ -59,6 +59,7 @@
        public function testChoicesFromDb( $name, $testCase ) {
                $this->setMwGlobals( 'wgCentralDBname', wfWikiID() );
 
+               $this->cnFixtures->prepareTestcase( $testCase );
                $this->cnFixtures->setupTestCase( $testCase['setup'] );
 
                $choices = $this->getProvider()->getChoicesForTesting( 
$this->rlContext );
diff --git a/tests/CentralNoticeTestFixtures.php 
b/tests/CentralNoticeTestFixtures.php
index a5a96e1..9e153d7 100644
--- a/tests/CentralNoticeTestFixtures.php
+++ b/tests/CentralNoticeTestFixtures.php
@@ -28,8 +28,8 @@
                );
                static::$defaultBanner = array(
                        'body' => 'testing',
-                       'displayAnon' => 
ApiCentralNoticeAllocations::DEFAULT_ANONYMOUS === 'true',
-                       'displayAccount' => 
ApiCentralNoticeAllocations::DEFAULT_ANONYMOUS === 'false',
+                       'displayAnon' => true,
+                       'displayAccount' => true,
                        'fundraising' => 1,
                        'autolink' => 0,
                        'landingPages' => 'JA1, JA2',
@@ -54,6 +54,48 @@
                return 'desktop';
        }
 
+       /**
+        *
+        * @see bannerController.lib.tests.js
+        *
+        * @param TS_UNIX $testCase
+        */
+       function prepareTestcase( &$testCase ) {
+               $now = wfTimestamp();
+
+               foreach ( $testCase['setup']['campaigns'] as &$campaign ) {
+
+                       $start = CentralNoticeTestFixtures::makeTimestamp(
+                               $now, $campaign['startDaysFromNow'] );
+
+                       $campaign['startTs'] = wfTimestamp( TS_MW, $start );
+
+                       $end = CentralNoticeTestFixtures::makeTimestamp(
+                                       $now, $campaign['endDaysFromNow'] );
+
+                       $campaign['endTs'] = wfTimestamp( TS_MW, $end );
+               }
+
+               foreach ( $testCase['choices'] as &$choice ) {
+
+                       $choice['start'] = 
CentralNoticeTestFixtures::makeTimestamp(
+                                       $now, $choice['startDaysFromNow'] );
+
+                       $choice['end'] = 
CentralNoticeTestFixtures::makeTimestamp(
+                                       $now, $choice['endDaysFromNow'] );
+
+                       // Unset these special properties from choices, for 
tests that
+                       // compare fixture choices to actual choices produced 
by the code
+                       // under test.
+                       unset( $choice['startDaysFromNow'] );
+                       unset( $choice['endDaysFromNow'] );
+               }
+       }
+
+       private static function makeTimestamp( $now, $offsetInDays ) {
+               return $now + ( 86400 * $offsetInDays );
+       }
+
        function setupTestCase( $spec ) {
                $this->ensureDesktopDevice();
 
@@ -75,6 +117,15 @@
                                $this->user
                        );
 
+                       // Update notice end date only if that property was 
sent in.
+                       // It may not be there since it's not in the defaults; 
not adding
+                       // since defaults will soon be removed (for json-based 
test
+                       // fixtures).
+                       if ( isset( $campaign['endTs'] ) ) {
+                               Campaign::updateNoticeDate( $campaign['name'],
+                                       $campaign['startTs'], 
$campaign['endTs'] );
+                       }
+
                        $banners = array();
                        foreach ( $campaign['banners'] as $bannerSpec ) {
                                $banner = $bannerSpec + static::$defaultBanner 
+ array(
diff --git a/tests/data/AllocationsFixtures.json 
b/tests/data/AllocationsFixtures.json
index 6e239d6..be5d166 100644
--- a/tests/data/AllocationsFixtures.json
+++ b/tests/data/AllocationsFixtures.json
@@ -4,6 +4,9 @@
             "setup": {
                 "campaigns": [
                     {
+                        "name": "c1",
+                        "startDaysFromNow": 0,
+                        "endDaysFromNow": 1,
                         "category": "fundraising",
                         "preferred": 1,
                         "throttle": 50,
@@ -24,25 +27,30 @@
             },
             "choices": [
                 {
+                    "name": "c1",
+                    "startDaysFromNow": 0,
+                    "endDaysFromNow": 1,
                     "preferred": 1,
                     "throttle": 50,
                     "bucket_count": 1,
                     "geotargeted": true,
-                    "countries": [
-                        "FR",
-                        "GR",
-                        "XX"
-                    ],
                     "banners": [
                         {
                             "name": "b1",
-                            "weight": 5,
                             "bucket": 0,
+                            "weight": 5,
                             "category": "fundraising",
+                            "display_anon": true,
+                            "display_account": true,
                             "devices": [
                                 "desktop"
                             ]
                         }
+                    ],
+                    "countries": [
+                        "FR",
+                        "GR",
+                        "XX"
                     ]
                 }
             ],
@@ -56,6 +64,8 @@
                 "campaigns": [
                     {
                         "name": "c1",
+                        "startDaysFromNow": 0,
+                        "endDaysFromNow": 1,
                         "preferred": 3,
                         "throttle": 60,
                         "banners": [
@@ -65,6 +75,8 @@
                     },
                     {
                         "name": "c2",
+                        "startDaysFromNow": 0,
+                        "endDaysFromNow": 1,
                         "preferred": 1,
                         "throttle": 100,
                         "banners": [
@@ -77,36 +89,67 @@
             "choices": [
                 {
                     "name": "c1",
-                    "geotargeted": false,
+                    "startDaysFromNow": 0,
+                    "endDaysFromNow": 1,
                     "preferred": 3,
                     "throttle": 60,
+                    "bucket_count": 1,
+                    "geotargeted": false,
                     "banners": [
                         {
                             "name": "c1b1",
+                            "bucket": 0,
                             "weight": 25,
-                            "category": "fundraising"
+                            "category": "fundraising",
+                            "display_anon": true,
+                            "display_account": true,
+                            "devices": [
+                                "desktop"
+                            ]
                         },
                         {
                             "name": "c1b2",
+                            "bucket": 0,
                             "weight": 25,
-                            "category": "fundraising"
+                            "category": "fundraising",
+                            "display_anon": true,
+                            "display_account": true,
+                            "devices": [
+                                "desktop"
+                            ]
                         }
                     ]
                 },
                 {
                     "name": "c2",
+                    "startDaysFromNow": 0,
+                    "endDaysFromNow": 1,
                     "preferred": 1,
                     "throttle": 100,
+                    "bucket_count": 1,
+                    "geotargeted": false,
                     "banners": [
                         {
                             "name": "c2b1",
+                            "bucket": 0,
                             "weight": 25,
-                            "category": "fundraising"
+                            "category": "fundraising",
+                            "display_anon": true,
+                            "display_account": true,
+                            "devices": [
+                                "desktop"
+                            ]
                         },
                         {
                             "name": "c2b2",
+                            "bucket": 0,
                             "weight": 25,
-                            "category": "fundraising"
+                            "category": "fundraising",
+                            "display_anon": true,
+                            "display_account": true,
+                            "devices": [
+                                "desktop"
+                            ]
                         }
                     ]
                 }
@@ -123,6 +166,9 @@
             "setup": {
                 "campaigns": [
                     {
+                        "name": "c1",
+                        "startDaysFromNow": 0,
+                        "endDaysFromNow": 1,
                         "banners": [
                             {
                                 "name": "b1",
@@ -142,19 +188,46 @@
             },
             "choices": [
                 {
+                    "name": "c1",
+                    "startDaysFromNow": 0,
+                    "endDaysFromNow": 1,
+                    "preferred": 1,
                     "throttle": 100,
+                    "bucket_count": 1,
+                    "geotargeted": false,
                     "banners": [
                         {
                             "name": "b1",
-                            "weight": 5
+                            "bucket": 0,
+                            "weight": 5,
+                            "category": "fundraising",
+                            "display_anon": true,
+                            "display_account": true,
+                            "devices": [
+                                "desktop"
+                            ]
                         },
                         {
                             "name": "b2",
-                            "weight": 100
+                            "bucket": 0,
+                            "weight": 100,
+                            "category": "fundraising",
+                            "display_anon": true,
+                            "display_account": true,
+                            "devices": [
+                                "desktop"
+                            ]
                         },
                         {
                             "name": "b3",
-                            "weight": 100
+                            "bucket": 0,
+                            "weight": 100,
+                            "category": "fundraising",
+                            "display_anon": true,
+                            "display_account": true,
+                            "devices": [
+                                "desktop"
+                            ]
                         }
                     ]
                 }
@@ -170,6 +243,9 @@
             "setup": {
                 "campaigns": [
                     {
+                        "name": "c1",
+                        "startDaysFromNow": 0,
+                        "endDaysFromNow": 1,
                         "throttle": 10,
                         "banners": [
                             { "name": "b1" }
@@ -179,9 +255,25 @@
             },
             "choices": [
                 {
+                    "name": "c1",
+                    "startDaysFromNow": 0,
+                    "endDaysFromNow": 1,
+                    "preferred": 1,
                     "throttle": 10,
+                    "bucket_count": 1,
+                    "geotargeted": false,
                     "banners": [
-                        { "name": "b1" }
+                        {
+                            "name": "b1",
+                            "bucket": 0,
+                            "weight": 25,
+                            "category": "fundraising",
+                            "display_anon": true,
+                            "display_account": true,
+                            "devices": [
+                                "desktop"
+                            ]
+                        }
                     ]
                 }
             ],
@@ -195,6 +287,8 @@
                 "campaigns": [
                     {
                         "name": "c1",
+                        "startDaysFromNow": 0,
+                        "endDaysFromNow": 1,
                         "preferred": 1,
                         "banners": [
                             { "name": "c1b1" }
@@ -202,6 +296,8 @@
                     },
                     {
                         "name": "c2",
+                        "startDaysFromNow": 0,
+                        "endDaysFromNow": 1,
                         "preferred": 2,
                         "banners": [
                             { "name": "c2b1" }
@@ -212,16 +308,46 @@
             "choices": [
                 {
                     "name": "c1",
+                    "startDaysFromNow": 0,
+                    "endDaysFromNow": 1,
                     "preferred": 1,
+                    "throttle": 100,
+                    "bucket_count": 1,
+                    "geotargeted": false,
                     "banners": [
-                        { "name": "c1b1" }
+                        {
+                            "name": "c1b1",
+                            "bucket": 0,
+                            "weight": 25,
+                            "category": "fundraising",
+                            "display_anon": true,
+                            "display_account": true,
+                            "devices": [
+                                "desktop"
+                            ]
+                        }
                     ]
                 },
                 {
                     "name": "c2",
+                    "startDaysFromNow": 0,
+                    "endDaysFromNow": 1,
                     "preferred": 2,
+                    "throttle": 100,
+                    "bucket_count": 1,
+                    "geotargeted": false,
                     "banners": [
-                        { "name": "c2b1" }
+                        {
+                            "name": "c2b1",
+                            "bucket": 0,
+                            "weight": 25,
+                            "category": "fundraising",
+                            "display_anon": true,
+                            "display_account": true,
+                            "devices": [
+                                "desktop"
+                            ]
+                        }
                     ]
                 }
             ],
@@ -235,6 +361,9 @@
             "setup": {
                 "campaigns": [
                     {
+                        "name": "c1",
+                        "startDaysFromNow": 0,
+                        "endDaysFromNow": 1,
                         "geotargeted": true,
                         "geo_countries": [
                             "FR",
@@ -251,16 +380,29 @@
             },
             "choices": [
                 {
+                    "name": "c1",
+                    "startDaysFromNow": 0,
+                    "endDaysFromNow": 1,
+                    "preferred": 1,
+                    "throttle": 100,
+                    "bucket_count": 1,
                     "geotargeted": true,
-                    "countries": [
-                        "FR",
-                        "XX"
-                    ],
                     "banners": [
                         {
                             "name": "b1",
-                            "weight": 5
+                            "weight": 5,
+                            "bucket": 0,
+                            "category": "fundraising",
+                            "display_anon": true,
+                            "display_account": true,
+                            "devices": [
+                                "desktop"
+                            ]
                         }
+                    ],
+                    "countries": [
+                        "FR",
+                        "XX"
                     ]
                 }
             ],
@@ -273,6 +415,9 @@
             "setup": {
                 "campaigns": [
                     {
+                        "name": "c1",
+                        "startDaysFromNow": 0,
+                        "endDaysFromNow": 1,
                         "geotargeted": true,
                         "geo_countries": [
                             "FR",
@@ -289,16 +434,29 @@
             },
             "choices": [
                 {
+                    "name": "c1",
+                    "startDaysFromNow": 0,
+                    "endDaysFromNow": 1,
+                    "preferred": 1,
+                    "throttle": 100,
+                    "bucket_count": 1,
                     "geotargeted": true,
-                    "countries": [
-                        "FR",
-                        "GR"
-                    ],
                     "banners": [
                         {
                             "name": "b1",
-                            "weight": 5
+                            "bucket": 0,
+                            "weight": 5,
+                            "category": "fundraising",
+                            "display_anon": true,
+                            "display_account": true,
+                            "devices": [
+                                "desktop"
+                            ]
                         }
+                    ],
+                    "countries": [
+                        "FR",
+                        "GR"
                     ]
                 }
             ],
@@ -310,6 +468,9 @@
             "setup": {
                 "campaigns": [
                     {
+                        "name": "c1",
+                        "startDaysFromNow": 0,
+                        "endDaysFromNow": 1,
                         "projects": [ "wikisource" ],
                         "banners": [
                             {
@@ -330,6 +491,9 @@
             "setup": {
                 "campaigns": [
                     {
+                        "name": "c1",
+                        "startDaysFromNow": 0,
+                        "endDaysFromNow": 1,
                         "project_languages": [ "zh" ],
                         "banners": [
                             {
@@ -350,7 +514,9 @@
             "setup": {
                 "campaigns": [
                     {
-                        "startTs": "20010101123456",
+                        "name": "c1",
+                        "startDaysFromNow": 1,
+                        "endDaysFromNow": 2,
                         "banners": [
                             {
                                 "name": "b1",
@@ -370,6 +536,9 @@
             "setup": {
                 "campaigns": [
                     {
+                        "name": "c1",
+                        "startDaysFromNow": 0,
+                        "endDaysFromNow": 1,
                         "banners": [
                             {
                                 "displayAnon": false,
@@ -383,12 +552,24 @@
             },
             "choices": [
                 {
+                    "name": "c1",
+                    "startDaysFromNow": 0,
+                    "endDaysFromNow": 1,
+                    "preferred": 1,
+                    "throttle": 100,
+                    "bucket_count": 1,
+                    "geotargeted": false,
                     "banners": [
                         {
                             "name": "b1",
+                            "bucket": 0,
                             "weight": 5,
+                            "category": "fundraising",
                             "display_anon": false,
-                            "display_account": true
+                            "display_account": true,
+                            "devices": [
+                                "desktop"
+                            ]
                         }
                     ]
                 }
diff --git a/tests/data/allocation_fixtures_doc.md 
b/tests/data/allocation_fixtures_doc.md
new file mode 100644
index 0000000..0889602
--- /dev/null
+++ b/tests/data/allocation_fixtures_doc.md
@@ -0,0 +1,14 @@
+Allocation Fixtures Documentation
+=================================
+
+Documentation for AllocationFixtures.json
+
+TODO: Complete this documentation
+
+
+Test Cases
+----------
+
+In the allocation fixture data, times are represented as offsets in days from 
the
+current time. When the fixtures are loaded, these are converted to UNIX
+timestamps (the format used by the code being tested).
\ No newline at end of file
diff --git 
a/tests/qunit/ext.centralNotice.bannerController.lib/bannerController.lib.tests.js
 
b/tests/qunit/ext.centralNotice.bannerController.lib/bannerController.lib.tests.js
index a18b257..cf808c4 100644
--- 
a/tests/qunit/ext.centralNotice.bannerController.lib/bannerController.lib.tests.js
+++ 
b/tests/qunit/ext.centralNotice.bannerController.lib/bannerController.lib.tests.js
@@ -1,21 +1,6 @@
 ( function( mw, $ ) {
        'use strict';
 
-       var defaultCampaignData = {
-               banners: [],
-               bucket_count: 1,
-               countries: [],
-               geotargeted: 0,
-               preferred: 1,
-               throttle: 100
-       }, defaultBannerData = {
-               bucket: 0,
-               devices: [ 'desktop' ],
-               weight: 25,
-               display_anon: true,
-               display_account: true
-       };
-
        QUnit.module( 'ext.centralNotice.bannerController.lib', 
QUnit.newMwEnvironment( {
                setup: function() {
                        mw.centralNotice.data.country = 'XX';
@@ -38,19 +23,10 @@
                                i,
                                allocatedBanner;
 
-                       // Flesh out choice data with some default values
-                       // BOOM on priority case
-                       choices = $.map( testCase.choices, function( campaign ) 
{
-                               return $.extend(
-                                       { },
-                                       defaultCampaignData,
-                                       campaign,
-                                       {
-                                               banners: $.map( 
campaign.banners, function( banner ) {
-                                                       return $.extend( {}, 
defaultBannerData, banner );
-                                               } )
-                                       } );
-                       } );
+                       // BOOM on priority case FIXME ???
+
+                       prepareTestCase( testCase );
+                       choices = testCase.choices;
 
                        // Set per-campaign buckets to 0 for all campaigns
                        // FIXME Allow testing of different buckets
@@ -91,6 +67,39 @@
                } );
        } );
 
+       /**
+        * Prepare a test case for use in a test. Currently just substitutes 
UNIX
+        * timestamps for times in the fixtures, which are represented as 
offsets
+        * in days from the current time. Note: this logic is repeated in PHP 
for
+        * PHPUnit tests that use the same fixtures.
+        *
+        * @see CentralNoticeTestFixtures::prepareTestcase()
+        */
+       function prepareTestCase( testCase ) {
+               var i, choice,
+                       now = new Date();
+
+               for ( i = 0; i < testCase.choices.length; i++ ) {
+                       choice = testCase.choices[i];
+                       choice.start = makeTimestamp( now, 
choice.startDaysFromNow );
+                       choice.end = makeTimestamp( now, choice.endDaysFromNow);
+               }
+       }
+
+       /**
+        * Return a UNIX timestamp for refDate offset by the number of days
+        * indicated.
+        *
+        * @param refDate Date The date to calculate the offset from
+        * @param offsetInDays
+        * @return int
+        */
+       function makeTimestamp( refDate, offsetInDays ) {
+               var date = new Date();
+               date.setDate( refDate.getDate() + offsetInDays );
+               return Math.round( date.getTime() / 1000 );
+       }
+
        // TODO: chooser tests
 
 } ( mediaWiki, jQuery ) );

-- 
To view, visit https://gerrit.wikimedia.org/r/182695
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I1e1e807deb86806f9c19e42dabeb857529e43e22
Gerrit-PatchSet: 7
Gerrit-Project: mediawiki/extensions/CentralNotice
Gerrit-Branch: master
Gerrit-Owner: AndyRussG <[email protected]>
Gerrit-Reviewer: AndyRussG <[email protected]>
Gerrit-Reviewer: Awight <[email protected]>
Gerrit-Reviewer: Ejegg <[email protected]>
Gerrit-Reviewer: Katie Horn <[email protected]>
Gerrit-Reviewer: Mwalker <[email protected]>
Gerrit-Reviewer: Ssmith <[email protected]>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to