Issue 5827 - Assign new groups even if prior groups are present

sitescripts/notifications/test/notification.py

 # This file is part of the Adblock Plus web scripts,
 # Copyright (C) 2006-present eyeo GmbH
 #
 # Adblock Plus is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License version 3 as
 # published by the Free Software Foundation.
 #
 # Adblock Plus is distributed in the hope that it will be useful,
 # but WITHOUT ANY WARRANTY; without even the implied warranty of
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 # GNU General Public License for more details.
 #
 # You should have received a copy of the GNU General Public License
 # along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
 
+import os
 import json
 import mock
 import unittest
+import tempfile
+import shutil
+import time
+import subprocess
+
+from sitescripts.utils import get_config
 
 import sitescripts.notifications.web.notification as notification
 
+mock_time = mock.Mock()
+mock_time.return_value = time.gmtime(0)
+
 
 class TestNotification(unittest.TestCase):
     def setUp(self):
-        self.load_notifications_patcher = mock.patch('sitescripts.notifications.web.notification.load_notifications')
-        self.load_notifications_mock = self.load_notifications_patcher.start()
+        self.repo_dir = tempfile.mkdtemp('md_repo')

[wspee, 2017/10/09 09:12:03]

As discussed: This created a new repository for every test. This make sense in
terms of test isolation but it means that this is quite slow (whole suit ~8s). I
explored different options to make it faster (hglib, copy repo instead of
creating a new one) but the main performance decrease comes from making commits
so I have abandoned the idea.

[Vasily Kuznetsov, 2017/10/09 22:29:11]

Hm. Now after I looked at the whole test suite it seems like mocking
`load_notifications` out in this file is the right approach, since we have a
whole separate set of tests for it in `parser.py` (and those tests actually use
a pretty cool way to quickly simulate `hg`, which I will neither endorse nor
disavow). So if there's something that `load_notifications` fails to do, we
should add a test for that to `parser.py`, not to here. Does this make sense?

[wspee, 2017/10/10 09:03:53]

No worries. Half of it was based on my assumptions anyways :). I see your point.
Done.

+        subprocess.check_call(['hg', 'init'], cwd=self.repo_dir)
+
+        open(os.path.join(self.repo_dir, '.hgignore'), 'w+').close()
+
+        subprocess.check_call(
+            ['hg', 'commit', '-q', '-m "Initial commit"', '-A'],
+            cwd=self.repo_dir)
+
+        config = get_config()
+        self._notification_repository_orig = config.get(
+            'notifications', 'repository')
+        get_config().set('notifications', 'repository', self.repo_dir)
 
     def tearDown(self):
-        self.load_notifications_patcher.stop()
+        shutil.rmtree(self.repo_dir)
+        get_config().set(
+            'notifications', 'repository', self._notification_repository_orig)
+
+    def add_notifications(self, notifications):
+        for id_, content in notifications:
+            with open(os.path.join(self.repo_dir, id_), 'w+') as f:
+                f.write(content)
+                f.flush()
+
+            subprocess.check_call(
+                ['hg', 'commit', '-q', '-m', id_, '-A'],
+                cwd=self.repo_dir)
 
     def test_no_group(self):
-        self.load_notifications_mock.return_value = [
-            {'id': '1', 'title': {'en-US': ''}, 'message': {'en-US': ''}}
-        ]
+        self.add_notifications([
+            ('1', 'title.en-US = 1\nmessage.en-US = 1\n')])
+
         result = json.loads(notification.notification({}, lambda *args: None))
         self.assertEqual(len(result['notifications']), 1)
         self.assertEqual(result['notifications'][0]['id'], '1')
         self.assertFalse('-' in result['version'])
 
     def test_not_in_group(self):
-        self.load_notifications_mock.return_value = [
-            {'id': '1', 'title': {'en-US': ''}, 'message': {'en-US': ''}},
-            {'id': 'a', 'variants': [
-                {'title': {'en-US': ''}, 'message': {'en-US': ''}}
-            ]}
-        ]
-        result = json.loads(notification.notification({
-            'QUERY_STRING': 'lastVersion=197001010000-a/0'
-        }, lambda *args: None))
-        self.assertEqual(len(result['notifications']), 1)
-        self.assertEqual(result['notifications'][0]['id'], '1')
-        self.assertRegexpMatches(result['version'], r'-a/0')
+        self.add_notifications([
+            ('1', 'title.en-US = 1\nmessage.en-US = 1\n'),
+            ('2', '[1]\ntitle.en-US = 2\nmessage.en-US = 2\nsample = 1'),
+        ])
+
+        result = json.loads(notification.notification({
+            'QUERY_STRING': 'lastVersion=197001010000-2/0'
+        }, lambda *args: None))
+        self.assertEqual(len(result['notifications']), 1)
+        self.assertEqual(result['notifications'][0]['id'], '1')
+        self.assertRegexpMatches(result['version'], r'-2/0')
 
     def test_in_group(self):
-        self.load_notifications_mock.return_value = [
-            {'id': '1', 'title': {'en-US': ''}, 'message': {'en-US': ''}},
-            {'id': 'a', 'variants': [
-                {'title': {'en-US': ''}, 'message': {'en-US': ''}}
-            ]}
-        ]
-        result = json.loads(notification.notification({
-            'QUERY_STRING': 'lastVersion=197001010000-a/1'
-        }, lambda *args: None))
-        self.assertEqual(len(result['notifications']), 1)
-        self.assertEqual(result['notifications'][0]['id'], 'a')
-        self.assertRegexpMatches(result['version'], r'-a/1')
+        self.add_notifications([
+            ('1', 'title.en-US = 1\nmessage.en-US = 1\n'),
+            ('2', '[1]\ntitle.en-US = 2.1\nmessage.en-US = 2.1\nsample = 1'),
+        ])
+
+        result = json.loads(notification.notification({
+            'QUERY_STRING': 'lastVersion=197001010000-2/1'
+        }, lambda *args: None))
+        self.assertEqual(len(result['notifications']), 1)
+        self.assertEqual(result['notifications'][0]['id'], '2')
+        self.assertRegexpMatches(result['version'], r'-2/1')
 
     def test_not_in_one_of_many_groups(self):
-        self.load_notifications_mock.return_value = [
-            {'id': '1', 'title': {'en-US': ''}, 'message': {'en-US': ''}},
-            {'id': 'a', 'variants': [
-                {'title': {'en-US': ''}, 'message': {'en-US': ''}}
-            ]},
-            {'id': 'b', 'variants': [
-                {'title': {'en-US': ''}, 'message': {'en-US': ''}}
-            ]},
-            {'id': 'c', 'variants': [
-                {'title': {'en-US': ''}, 'message': {'en-US': ''}}
-            ]}
-        ]
-        result = json.loads(notification.notification({
-            'QUERY_STRING': 'lastVersion=197001010000-a/0-b/0-c/0'
-        }, lambda *args: None))
-        self.assertEqual(len(result['notifications']), 1)
-        self.assertEqual(result['notifications'][0]['id'], '1')
-        self.assertRegexpMatches(result['version'], r'-a/0-b/0-c/0')
+        self.add_notifications([
+            ('1', 'title.en-US = 1\nmessage.en-US = 1\n'),
+            ('2', '[1]\ntitle.en-US = 2.1\nmessage.en-US = 2.1\nsample = 1'),
+            ('3', '[1]\ntitle.en-US = 3.1\nmessage.en-US = 3.1\nsample = 1'),
+            ('4', '[1]\ntitle.en-US = 4.1\nmessage.en-US = 4.1\nsample = 1'),
+        ])
+
+        result = json.loads(notification.notification({
+            'QUERY_STRING': 'lastVersion=197001010000-2/0-3/0-4/0'
+        }, lambda *args: None))
+        self.assertEqual(len(result['notifications']), 1)
+        self.assertEqual(result['notifications'][0]['id'], '1')
+        self.assertRegexpMatches(result['version'], r'-2/0-3/0-4/0')
 
     def test_in_one_of_many_groups(self):
-        self.load_notifications_mock.return_value = [
-            {'id': '1', 'title': {'en-US': ''}, 'message': {'en-US': ''}},
-            {'id': 'a', 'variants': [
-                {'title': {'en-US': ''}, 'message': {'en-US': ''}}
-            ]},
-            {'id': 'b', 'variants': [
-                {'title': {'en-US': ''}, 'message': {'en-US': ''}}
-            ]},
-            {'id': 'c', 'variants': [
-                {'title': {'en-US': ''}, 'message': {'en-US': ''}}
-            ]}
-        ]
-        result = json.loads(notification.notification({
-            'QUERY_STRING': 'lastVersion=197001010000-a/0-b/1-c/0'
-        }, lambda *args: None))
-        self.assertEqual(len(result['notifications']), 1)
-        self.assertEqual(result['notifications'][0]['id'], 'b')
-        self.assertRegexpMatches(result['version'], r'-a/0-b/1-c/0')
+        self.add_notifications([
+            ('1', 'title.en-US = 1\nmessage.en-US = 1\n'),
+            ('2', '[1]\ntitle.en-US = 2.1\nmessage.en-US = 2.1\nsample = 1'),
+            ('3', '[1]\ntitle.en-US = 3.1\nmessage.en-US = 3.1\nsample = 1'),
+            ('4', '[1]\ntitle.en-US = 4.1\nmessage.en-US = 4.1\nsample = 1'),
+        ])
+
+        result = json.loads(notification.notification({
+            'QUERY_STRING': 'lastVersion=197001010000-2/0-3/1-4/0'
+        }, lambda *args: None))
+        self.assertEqual(len(result['notifications']), 1)
+        self.assertEqual(result['notifications'][0]['id'], '3')
+        self.assertRegexpMatches(result['version'], r'-2/0-3/1-4/0')
 
     def test_not_put_in_group(self):
-        self.load_notifications_mock.return_value = [
-            {'id': '1', 'title': {'en-US': ''}, 'message': {'en-US': ''}},
-            {'id': 'a', 'variants': [
-                {'sample': 0, 'title': {'en-US': ''}, 'message': {'en-US': ''}}
-            ]}
-        ]
+        self.add_notifications([

[Vasily Kuznetsov, 2017/10/09 22:29:11]

There is and was a lot of code duplication in this file. If you end up touching
these methods, it would probably be good to move those repeated data structures
out as constants or reuse them in some other way.

[wspee, 2017/10/10 09:03:53]

Acknowledged.

+            ('1', 'title.en-US = 1\nmessage.en-US = 1\n'),
+            ('2', '[1]\ntitle.en-US = 2.1\nmessage.en-US = 2.1\nsample = 0'),
+        ])
+
         result = json.loads(notification.notification({
             'QUERY_STRING': 'lastVersion=197001010000'
         }, lambda *args: None))
         self.assertEqual(len(result['notifications']), 1)
         self.assertEqual(result['notifications'][0]['id'], '1')
-        self.assertRegexpMatches(result['version'], r'-a/0')
+        self.assertRegexpMatches(result['version'], r'-2/0')
 
     def test_put_in_group(self):
-        self.load_notifications_mock.return_value = [
-            {'id': '1', 'title': {'en-US': ''}, 'message': {'en-US': ''}},
-            {'id': 'a', 'variants': [
-                {'sample': 1, 'title': {'en-US': ''}, 'message': {'en-US': ''}}
-            ]}
-        ]
+        self.add_notifications([
+            ('1', 'title.en-US = 1\nmessage.en-US = 1\n'),
+            ('2', '[1]\ntitle.en-US = 2.1\nmessage.en-US = 2.1\nsample = 1'),
+        ])
+
         result = json.loads(notification.notification({
             'QUERY_STRING': 'lastVersion=197001010000'
         }, lambda *args: None))
         self.assertEqual(len(result['notifications']), 1)
-        self.assertEqual(result['notifications'][0]['id'], 'a')
-        self.assertRegexpMatches(result['version'], r'-a/1')
-
-    def test_notification_variant_merged(self):

[wspee, 2017/10/09 09:12:03]

This "merging" is not possible with the notification parser so I had to remove
this test.

-        self.load_notifications_mock.return_value = [
-            {
-                'id': 'a',
-                'title': {'en-US': 'default'},
-                'message': {'en-US': 'default'},
-                'variants': [
-                    {'sample': 1, 'message': {'en-US': 'variant'}}
-                ]
-            }
-        ]
-        result = json.loads(notification.notification({}, lambda *args: None))
-        self.assertEqual(len(result['notifications']), 1)
-        self.assertEqual(result['notifications'][0]['id'], 'a')
-        self.assertEqual(result['notifications'][0]['title']['en-US'], 'default')
-        self.assertEqual(result['notifications'][0]['message']['en-US'], 'variant')
-        self.assertFalse('variants' in result['notifications'][0])
-        self.assertFalse('sample' in result['notifications'][0])
+        self.assertEqual(result['notifications'][0]['id'], '2')
+        self.assertRegexpMatches(result['version'], r'-2/1')
 
     def test_no_variant_no_notifications(self):
-        self.load_notifications_mock.return_value = [
-            {'id': 'a', 'variants': [{'sample': 0}]}
-        ]
+        self.add_notifications([
+            ('1', '[1]\ntitle.en-US = 1\nmessage.en-US = 1\nsample = 0'),
+        ])
+
         result = json.loads(notification.notification({}, lambda *args: None))
         self.assertEqual(len(result['notifications']), 0)
 
     @mock.patch('random.random')
     def test_probability_distribution_single_group(self, random_call):
-        self.load_notifications_mock.return_value = [
-            {
-                'id': 'a',
-                'variants': [
-                    {'sample': 0.5, 'title': {'en-US': '1'}, 'message': {'en-US': ''}},
-                    {'sample': 0.25, 'title': {'en-US': '2'}, 'message': {'en-US': ''}},
-                    {'sample': 0.25, 'title': {'en-US': '3'}, 'message': {'en-US': ''}}
-                ]
-            }
-        ]
+        self.add_notifications([
+            ('1', '[1]\n'
+                  'title.en-US = 1.1\n'
+                  'message.en-US = 1.1\n'
+                  'sample = 0.5\n'
+                  '[2]\n'
+                  'title.en-US = 1.2\n'
+                  'message.en-US = 1.2\n'
+                  'sample = 0.25\n'
+                  '[3]\n'
+                  'title.en-US = 1.3\n'
+                  'message.en-US = 1.3\n'
+                  'sample = 0.25\n')
+        ])
+
         random_call.return_value = 0
         result = json.loads(notification.notification({}, lambda *args: None))
         self.assertEqual(len(result['notifications']), 1)
-        self.assertEqual(result['notifications'][0]['title']['en-US'], '1')
-        self.assertRegexpMatches(result['version'], r'-a/1')
+        self.assertEqual(result['notifications'][0]['title']['en-US'], '1.1')
+        self.assertRegexpMatches(result['version'], r'-1/1')
         random_call.return_value = 0.5
         result = json.loads(notification.notification({}, lambda *args: None))
         self.assertEqual(len(result['notifications']), 1)
-        self.assertEqual(result['notifications'][0]['title']['en-US'], '1')
-        self.assertRegexpMatches(result['version'], r'-a/1')
+        self.assertEqual(result['notifications'][0]['title']['en-US'], '1.1')
+        self.assertRegexpMatches(result['version'], r'-1/1')
         random_call.return_value = 0.51
         result = json.loads(notification.notification({}, lambda *args: None))
         self.assertEqual(len(result['notifications']), 1)
-        self.assertEqual(result['notifications'][0]['title']['en-US'], '2')
-        self.assertRegexpMatches(result['version'], r'-a/2')
+        self.assertEqual(result['notifications'][0]['title']['en-US'], '1.2')
+        self.assertRegexpMatches(result['version'], r'-1/2')
         random_call.return_value = 0.75
         result = json.loads(notification.notification({}, lambda *args: None))
         self.assertEqual(len(result['notifications']), 1)
-        self.assertEqual(result['notifications'][0]['title']['en-US'], '2')
-        self.assertRegexpMatches(result['version'], r'-a/2')
+        self.assertEqual(result['notifications'][0]['title']['en-US'], '1.2')
+        self.assertRegexpMatches(result['version'], r'-1/2')
         random_call.return_value = 0.751
         result = json.loads(notification.notification({}, lambda *args: None))
         self.assertEqual(len(result['notifications']), 1)
-        self.assertEqual(result['notifications'][0]['title']['en-US'], '3')
-        self.assertRegexpMatches(result['version'], r'-a/3')
+        self.assertEqual(result['notifications'][0]['title']['en-US'], '1.3')
+        self.assertRegexpMatches(result['version'], r'-1/3')
         random_call.return_value = 1
         result = json.loads(notification.notification({}, lambda *args: None))
         self.assertEqual(len(result['notifications']), 1)
-        self.assertEqual(result['notifications'][0]['title']['en-US'], '3')
-        self.assertRegexpMatches(result['version'], r'-a/3')
+        self.assertEqual(result['notifications'][0]['title']['en-US'], '1.3')
+        self.assertRegexpMatches(result['version'], r'-1/3')
 
     @mock.patch('random.random')
     def test_probability_distribution_multiple_groups(self, random_call):
-        self.load_notifications_mock.return_value = [
-            {
-                'id': 'a',
-                'variants': [
-                    {'sample': 0.25, 'title': {'en-US': '1'}, 'message': {'en-US': ''}},
-                    {'sample': 0.25, 'title': {'en-US': '2'}, 'message': {'en-US': ''}}
-                ]
-            },
-            {
-                'id': 'b',
-                'variants': [
-                    {'sample': 0.25, 'title': {'en-US': '1'}, 'message': {'en-US': ''}},
-                    {'sample': 0.25, 'title': {'en-US': '2'}, 'message': {'en-US': ''}}
-                ]
-            }
-        ]
+        self.add_notifications([
+            ('1', '[1]\n'
+                  'title.en-US = 1.1\n'
+                  'message.en-US = 1.1\n'
+                  'sample = 0.25\n'
+                  '[2]\n'
+                  'title.en-US = 1.2\n'
+                  'message.en-US = 1.2\n'
+                  'sample = 0.25\n'),
+            ('2', '[1]\n'
+                  'title.en-US = 2.1\n'
+                  'message.en-US = 2.1\n'
+                  'sample = 0.25\n'
+                  '[2]\n'
+                  'title.en-US = 2.2\n'
+                  'message.en-US = 2.1\n'
+                  'sample = 0.25\n')
+        ])
+
         random_call.return_value = 0
         result = json.loads(notification.notification({}, lambda *args: None))
         self.assertEqual(len(result['notifications']), 1)
-        self.assertEqual(result['notifications'][0]['id'], 'a')
-        self.assertEqual(result['notifications'][0]['title']['en-US'], '1')
-        self.assertRegexpMatches(result['version'], r'-a/1-b/0')
+        self.assertEqual(result['notifications'][0]['id'], '1')
+        self.assertEqual(result['notifications'][0]['title']['en-US'], '1.1')
+        self.assertRegexpMatches(result['version'], r'-1/1-2/0')
         random_call.return_value = 0.251
         result = json.loads(notification.notification({}, lambda *args: None))
         self.assertEqual(len(result['notifications']), 1)
-        self.assertEqual(result['notifications'][0]['id'], 'a')
-        self.assertEqual(result['notifications'][0]['title']['en-US'], '2')
-        self.assertRegexpMatches(result['version'], r'-a/2-b/0')
+        self.assertEqual(result['notifications'][0]['id'], '1')
+        self.assertEqual(result['notifications'][0]['title']['en-US'], '1.2')
+        self.assertRegexpMatches(result['version'], r'-1/2-2/0')
         random_call.return_value = 0.51
         result = json.loads(notification.notification({}, lambda *args: None))
         self.assertEqual(len(result['notifications']), 1)
-        self.assertEqual(result['notifications'][0]['id'], 'b')
-        self.assertEqual(result['notifications'][0]['title']['en-US'], '1')
-        self.assertRegexpMatches(result['version'], r'-a/0-b/1')
+        self.assertEqual(result['notifications'][0]['id'], '2')
+        self.assertEqual(result['notifications'][0]['title']['en-US'], '2.1')
+        self.assertRegexpMatches(result['version'], r'-1/0-2/1')
         random_call.return_value = 0.751
         result = json.loads(notification.notification({}, lambda *args: None))
         self.assertEqual(len(result['notifications']), 1)
-        self.assertEqual(result['notifications'][0]['id'], 'b')
-        self.assertEqual(result['notifications'][0]['title']['en-US'], '2')
-        self.assertRegexpMatches(result['version'], r'-a/0-b/2')
-
+        self.assertEqual(result['notifications'][0]['id'], '2')
+        self.assertEqual(result['notifications'][0]['title']['en-US'], '2.2')
+        self.assertRegexpMatches(result['version'], r'-1/0-2/2')
+
+    @mock.patch('time.gmtime', mock_time)
     def test_invalid_last_version(self):
-        self.load_notifications_mock.return_value = []
         notification.notification({'QUERY_STRING': 'lastVersion='},
                                   lambda *args: None)
+        result = json.loads(notification.notification({}, lambda *args: None))
+        self.assertEqual(result['version'], '197001010000')
+
         notification.notification({'QUERY_STRING': 'lastVersion=-'},
                                   lambda *args: None)
+        result = json.loads(notification.notification({}, lambda *args: None))
+        self.assertEqual(result['version'], '197001010000')
+
         notification.notification({'QUERY_STRING': 'lastVersion=-/'},
                                   lambda *args: None)
+        result = json.loads(notification.notification({}, lambda *args: None))
+        self.assertEqual(result['version'], '197001010000')
+
         notification.notification({'QUERY_STRING': 'lastVersion=-//'},
                                   lambda *args: None)
+        result = json.loads(notification.notification({}, lambda *args: None))
+        self.assertEqual(result['version'], '197001010000')
 
     def test_version_header_present(self):
-        self.load_notifications_mock.return_value = [
-            {'id': '1', 'title': {'en-US': ''}, 'message': {'en-US': ''}}
-        ]
+        self.add_notifications([
+            ('1', 'title.en-US = 1\nmessage.en-US = 1\n')
+        ])
         response_header_map = {}
 
         def start_response(status, response_headers):
             for name, value in response_headers:
                 response_header_map[name] = value
         result = json.loads(notification.notification({}, start_response))
         self.assertEqual(result['version'],
                          response_header_map['ABP-Notification-Version'])
 
     def test_default_group_notification_returned_if_valid(self):
-        self.load_notifications_mock.return_value = [
-            {'id': '1', 'title': {'en-US': ''}, 'message': {'en-US': ''}},
-            {
-                'id': 'a',
-                'title': {'en-US': '0'},
-                'message': {'en-US': '0'},
-                'variants': [
-                    {'title': {'en-US': '1'}, 'message': {'en-US': '1'}}
-                ]
-            }
-        ]
+        self.add_notifications([
+            ('1', 'title.en-US = 1\nmessage.en-US = 1\n'),
+            ('2', 'title.en-US = 2\nmessage.en-US = 2\n'
+                  '[1]\ntitle.en-US = 2.1\nmessage.en-US = 2.1'),
+        ])
+
         result = json.loads(notification.notification({
-            'QUERY_STRING': 'lastVersion=197001010000-a/0'
+            'QUERY_STRING': 'lastVersion=197001010000-2/0'
         }, lambda *args: None))
         self.assertEqual(len(result['notifications']), 2)
         self.assertEqual(result['notifications'][0]['id'], '1')
-        self.assertEqual(result['notifications'][1]['id'], 'a')
-        self.assertEqual(result['notifications'][1]['title']['en-US'], '0')
+        self.assertEqual(result['notifications'][1]['id'], '2')
+        self.assertEqual(result['notifications'][1]['title']['en-US'], '2')
         self.assertNotIn('variants', result['notifications'][1])
-        self.assertRegexpMatches(result['version'], r'-a/0')
+        self.assertRegexpMatches(result['version'], r'-2/0')
 
     def test_default_group_notification_not_returned_if_invalid(self):
-        self.load_notifications_mock.return_value = [
-            {'id': '1', 'title': {'en-US': ''}, 'message': {'en-US': ''}},
-            {
-                'id': 'a',
-                'title': {'en-US': '0'},
-                'variants': [
-                    {'title': {'en-US': '1'}, 'message': {'en-US': '1'}}
-                ]
-            }
-        ]
+        self.add_notifications([
+            ('1', 'title.en-US = 1\nmessage.en-US = 1\n'),
+            ('2', 'title.en-US = 2\n'
+                  '[1]\ntitle.en-US = 2.1\nmessage.en-US = 2.1'),
+        ])
         result = json.loads(notification.notification({
-            'QUERY_STRING': 'lastVersion=197001010000-a/0'
+            'QUERY_STRING': 'lastVersion=197001010000-2/0'
         }, lambda *args: None))
         self.assertEqual(len(result['notifications']), 1)
         self.assertEqual(result['notifications'][0]['id'], '1')
-        self.assertRegexpMatches(result['version'], r'-a/0')
+        self.assertRegexpMatches(result['version'], r'-2/0')
 
     def test_invalid_notification_not_returned(self):
-        self.load_notifications_mock.return_value = [
-            {'id': '1', 'title': {'en-US': ''}, 'message': {'en-US': ''}},
-            {'id': '2', 'title': {'en-US': ''}, 'message': {}},

[wspee, 2017/10/09 09:12:03]

This is not possible with the parser, so I removed the notification from the new
test.

-            {'id': '3', 'title': {}, 'message': {'en-US': ''}},
-            {'id': '4', 'title': {}},
-            {'id': '5', 'message': {}},
-            {'id': '6'}
-        ]
+        self.add_notifications([
+            ('1', 'title.en-US = 1\nmessage.en-US = 1\n'),
+            ('2', 'title.en-US = 2\n'),
+            ('3', 'title.en-US = \n'),
+            ('4', 'message.en-US = \n'),
+            ('5', '\n'),
+        ])
+
         result = json.loads(notification.notification({}, lambda *args: None))
         self.assertEqual(len(result['notifications']), 1)
         self.assertEqual(result['notifications'][0]['id'], '1')
 
     def test_stays_in_group_when_notification_present(self):
-        self.load_notifications_mock.return_value = [
-            {'id': 'a'}
-        ]
+        self.add_notifications([
+            ('1', '\n'),
+        ])
         result = json.loads(notification.notification({
-            'QUERY_STRING': 'lastVersion=197001010000-a/0-b/1'
+            'QUERY_STRING': 'lastVersion=197001010000-1/0-2/1'
         }, lambda *args: None))
         self.assertEqual(len(result['notifications']), 0)
-        self.assertRegexpMatches(result['version'], r'-a/0')
+        self.assertRegexpMatches(result['version'], r'-1/0')
 
     def test_leaves_group_when_notification_absent(self):
-        self.load_notifications_mock.return_value = []
         result = json.loads(notification.notification({
-            'QUERY_STRING': 'lastVersion=197001010000-a/0-b/1'
+            'QUERY_STRING': 'lastVersion=197001010000-1/0-2/1'
         }, lambda *args: None))
         self.assertEqual(len(result['notifications']), 0)
         self.assertRegexpMatches(result['version'], r'[^-]*')
 
     def test_stays_in_group_when_notification_inactive(self):
-        self.load_notifications_mock.return_value = [
-            {'id': 'a', 'inactive': True}
-        ]
+        self.add_notifications(
+            [('1', 'inactive = true\ntitle.en-US = 1\nmessage.en-US = 1\n')])
         result = json.loads(notification.notification({
-            'QUERY_STRING': 'lastVersion=197001010000-a/0-b/1'
+            'QUERY_STRING': 'lastVersion=197001010000-1/0-2/1'
         }, lambda *args: None))
         self.assertEqual(len(result['notifications']), 0)
-        self.assertRegexpMatches(result['version'], r'-a/0')
+        self.assertRegexpMatches(result['version'], r'-1/0')
+
+    def test_stays_in_group_when_notification_inactive_assign_new_group(self):
+        # See: https://issues.adblockplus.org/ticket/5827
+        self.add_notifications([
+            ('1', 'inactive = true\ntitle.en-US = 1.1\nmessage.en-US = 1.1'),
+            ('2', '[1]\ntitle.en-US = 2.1\nmessage.en-US = 2.1\nsample = 1\n'),
+        ])
+        result = json.loads(notification.notification({
+            'QUERY_STRING': 'lastVersion=197001010000-1/0'
+        }, lambda *args: None))
+
+        self.assertEqual(len(result['notifications']), 1)
+        self.assertRegexpMatches(result['version'], r'-1/0-2/1')
 
     def test_inactive_notifications_not_returned(self):
-        self.load_notifications_mock.return_value = [
-            {'id': 'a', 'title': {'en-US': ''}, 'message': {'en-US': ''}, 'inactive': True},
-            {'id': 'b', 'title': {'en-US': ''}, 'message': {'en-US': ''}, 'inactive': False},
-            {'id': 'c', 'title': {'en-US': ''}, 'message': {'en-US': ''}}
-        ]
+        self.add_notifications([
+            ('1', 'inactive = true\ntitle.en-US = 1\nmessage.en-US = 1\n'),
+            ('2', 'inactive = false\ntitle.en-US = 2\nmessage.en-US = 2\n'),
+            ('3', 'title.en-US = 3\nmessage.en-US = 3\n'),
+        ])
+
         result = json.loads(notification.notification({}, lambda *args: None))
         self.assertEqual(len(result['notifications']), 2)
-        self.assertEqual(result['notifications'][0]['id'], 'b')
-        self.assertEqual(result['notifications'][1]['id'], 'c')
+        self.assertEqual(result['notifications'][0]['id'], '2')
+        self.assertEqual(result['notifications'][1]['id'], '3')
 
     def test_inactive_notification_variant_not_returned(self):
-        self.load_notifications_mock.return_value = [
-            {'id': 'a', 'inactive': True}
-        ]
+        self.add_notifications([
+            ('1', 'inactive = true\ntitle.en-US = 1\nmessage.en-US = 1\n'),
+        ])
         result = json.loads(notification.notification({
             'QUERY_STRING': 'lastVersion=197001010000-a/1'
         }, lambda *args: None))
         self.assertEqual(len(result['notifications']), 0)
 
 if __name__ == '__main__':
     unittest.main()