cli tests

CHANGELOG.rst

@@ -14,6 +14,7 @@ Changelog
 **Bugfixes**
 
 - Fix bug that would exit update in singlerun mode when an error occurred during update
+- Fix bug that would not run after cleanup in cli mode
 
 **Improvements**
 * Added a **--mode** parameter which allows you to run the updater using a cron. You have to ensure that only one instance run at a time however (e.g. using SystemD timers)

owncloud_news_updater/api/cli.py

@@ -15,8 +15,10 @@ class CliApi(Api):
     def __init__(self, config):
         directory = config.url
         phpini = config.phpini
-        self.directory = directory.rstrip('/')
-        base_command = ['php', '-f', self.directory + '/occ']
+        if not directory.endswith('/'):
+            directory += '/'
+        self.directory = directory
+        base_command = ['php', '-f', self.directory + 'occ']
         if phpini is not None and phpini.strip() != '':
             base_command += ['-c', phpini]
         self.before_cleanup_command = base_command + [
@@ -55,8 +57,7 @@ class CliUpdater(Updater):
         self.cli.run(self.api.before_cleanup_command)
 
     def start_update_thread(self, feeds):
-        return CliUpdateThread(feeds, self.logger,
-                               self.api, self.cli)
+        return CliUpdateThread(feeds, self.logger, self.api, self.cli)
 
     def all_feeds(self):
         feeds_json = self.cli.run(self.api.all_feeds_command).strip()
@@ -69,7 +70,7 @@ class CliUpdater(Updater):
     def after_update(self):
         self.logger.info('Running after update command: %s' %
                          ' '.join(self.api.after_cleanup_command))
-        self.cli.run(self.api.before_cleanup_command)
+        self.cli.run(self.api.after_cleanup_command)
 
 
 class CliUpdateThread(UpdateThread):

owncloud_news_updater/api/web.py

@@ -1,6 +1,7 @@
 import base64
 import urllib.parse
 import urllib.request
+from collections import OrderedDict
 
 from owncloud_news_updater.api.api import Api, Feed
 from owncloud_news_updater.api.updater import Updater, UpdateThread
@@ -96,10 +97,12 @@ class WebUpdateThread(UpdateThread):
         self.config = config
 
     def update_feed(self, feed):
-        data = {
-            'feedId': feed.feed_id,
-            'userId': feed.user_id
-        }
+        # make sure that the order is always defined for making it easier
+        # to test and reason about, normal dicts are not ordered
+        data = OrderedDict([
+            ('userId', feed.user_id),
+            ('feedId', feed.feed_id),
+        ])
         data = urllib.parse.urlencode(data)
         url = '%s?%s' % (self.api.update_url, data)
         self.logger.info('Calling update url: %s' % url)

tests/owncloud_news_updater/api/test_cli.py

@@ -0,0 +1,72 @@
+import json
+from unittest import TestCase
+from unittest.mock import MagicMock, call
+
+from owncloud_news_updater.api.updater import Updater
+from owncloud_news_updater.api.cli import Cli, CliApi, CliApiV2
+from owncloud_news_updater.config import Config
+from owncloud_news_updater.container import Container
+
+
+class TestCli(TestCase):
+    def setUp(self):
+        self.container = Container()
+        self.cli = MagicMock(spec=Cli)
+        self.container.register(Cli, lambda c: self.cli)
+        self.base_url = '/'
+
+    def _set_config(self, **kwargs):
+        config = Config()
+        for key, value in kwargs.items():
+            setattr(config, key, value)
+        self.container.register(Config, lambda c: config)
+
+    def _set_cli_run(self, return_value):
+        attrs = {'run.return_value': bytes(json.dumps(return_value), 'utf-8')}
+        self.cli.configure_mock(**attrs)
+
+    def test_api_level(self):
+        self._set_config(apilevel='v1-2', url=self.base_url)
+        api = self.container.resolve(CliApi)
+        self.assertIsInstance(api, CliApi)
+
+    def test_api_level_v2(self):
+        self._set_config(apilevel='v2', url=self.base_url)
+        api = self.container.resolve(CliApi)
+        self.assertIsInstance(api, CliApiV2)
+
+    def _create_commands(self):
+        base_cmd = ['php', '-f', '%socc' % self.base_url]
+        before_cmd = base_cmd + ['news:updater:before-update']
+        feeds_cmd = base_cmd + ['news:updater:all-feeds']
+        update_cmd1 = base_cmd + ['news:updater:update-feed', '2', 'deb']
+        update_cmd2 = base_cmd + ['news:updater:update-feed', '3', 'john']
+        after_cmd = base_cmd + ['news:updater:after-update']
+
+        # ordering can be switched due to threading, so try both cases
+        return ([call(before_cmd), call(feeds_cmd), call(update_cmd1),
+                 call(update_cmd2), call(after_cmd)],
+                [call(before_cmd), call(feeds_cmd), call(update_cmd2),
+                 call(update_cmd1), call(after_cmd)])
+
+    def test_api_v1_calls(self):
+        self._set_config(apilevel='v1-2', url=self.base_url, mode='singlerun')
+        updater = self.container.resolve(Updater)
+        self._set_cli_run({
+            'feeds': [{'id': 3, 'userId': 'john'}, {'id': 2, 'userId': 'deb'}]
+        })
+        updater.run()
+        self.assertIn(self.cli.run.call_args_list, self._create_commands())
+
+    def test_api_v2_calls(self):
+        self._set_config(apilevel='v2', url=self.base_url, mode='singlerun')
+        updater = self.container.resolve(Updater)
+        self._set_cli_run({
+            'data': {
+                'updater': [{'feedId': 3, 'userId': 'john'},
+                            {'feedId': 2, 'userId': 'deb'}]
+            }
+        })
+        updater.run()
+
+        self.assertIn(self.cli.run.call_args_list, self._create_commands())

tests/owncloud_news_updater/api/test_web.py

@@ -35,15 +35,7 @@ class TestWeb(TestCase):
         api = self.container.resolve(WebApi)
         self.assertIsInstance(api, WebApiV2)
 
-    def test_api_v1_calls(self):
-        self._set_config(apilevel='v1-2', url=self.base_url, user='john',
-                         password='pass', mode='singlerun')
-        updater = self.container.resolve(Updater)
-        self._set_http_get({
-            'feeds': [{'id': 3, 'userId': 'john'}, {'id': 2, 'userId': 'deb'}]
-        })
-        updater.run()
-
+    def _create_urls_v1(self):
         before_url = '%s/index.php/apps/news/api/v1-2/cleanup/before-update' \
                      % self.base_url
         feeds_url = '%s/index.php/apps/news/api/v1-2/feeds/all' \
@@ -57,25 +49,15 @@ class TestWeb(TestCase):
         auth = ('john', 'pass')
         timeout = 5 * 60
 
-        asserted_calls = [call(before_url, auth), call(feeds_url, auth),
+        # ordering can be switched due to threading, so try both cases
+        return ([call(before_url, auth), call(feeds_url, auth),
                  call(update_url1, auth, timeout),
+                 call(update_url2, auth, timeout), call(after_url, auth)],
+                [call(before_url, auth), call(feeds_url, auth),
                  call(update_url2, auth, timeout),
-                          call(after_url, auth)]
-
-        self.http.get.assert_has_calls(asserted_calls)
-
-    def test_api_v2_calls(self):
-        self._set_config(apilevel='v2', url=self.base_url, user='john',
-                         password='pass', mode='singlerun')
-        updater = self.container.resolve(Updater)
-        self._set_http_get({
-            'data': {
-                'updater': [{'feedId': 3, 'userId': 'john'},
-                            {'feedId': 2, 'userId': 'deb'}]
-            }
-        })
-        updater.run()
+                 call(update_url1, auth, timeout), call(after_url, auth)])
 
+    def _create_urls_v2(self):
         before_url = '%s/index.php/apps/news/api/v2/updater/before-update' \
                      % self.base_url
         feeds_url = '%s/index.php/apps/news/api/v2/updater/all-feeds' \
@@ -89,9 +71,33 @@ class TestWeb(TestCase):
         auth = ('john', 'pass')
         timeout = 5 * 60
 
-        asserted_calls = [call(before_url, auth), call(feeds_url, auth),
+        # ordering can be switched due to threading, so try both cases
+        return ([call(before_url, auth), call(feeds_url, auth),
                  call(update_url1, auth, timeout),
+                 call(update_url2, auth, timeout), call(after_url, auth)],
+                [call(before_url, auth), call(feeds_url, auth),
                  call(update_url2, auth, timeout),
-                          call(after_url, auth)]
+                 call(update_url1, auth, timeout), call(after_url, auth)])
 
-        self.http.get.assert_has_calls(asserted_calls)
+    def test_api_v1_calls(self):
+        self._set_config(apilevel='v1-2', url=self.base_url, user='john',
+                         password='pass', mode='singlerun')
+        updater = self.container.resolve(Updater)
+        self._set_http_get({
+            'feeds': [{'id': 3, 'userId': 'john'}, {'id': 2, 'userId': 'deb'}]
+        })
+        updater.run()
+        self.assertIn(self.http.get.call_args_list, self._create_urls_v1())
+
+    def test_api_v2_calls(self):
+        self._set_config(apilevel='v2', url=self.base_url, user='john',
+                         password='pass', mode='singlerun')
+        updater = self.container.resolve(Updater)
+        self._set_http_get({
+            'data': {
+                'updater': [{'feedId': 3, 'userId': 'john'},
+                            {'feedId': 2, 'userId': 'deb'}]
+            }
+        })
+        updater.run()
+        self.assertIn(self.http.get.call_args_list, self._create_urls_v2())