From 4a8c4cfc53db989b8db941d1b8fcd02c5e33d142 Mon Sep 17 00:00:00 2001
From: Alexander Kukushkin <cyberdemn@gmail.com>
Date: Fri, 15 Jan 2021 14:29:55 +0100
Subject: [PATCH] Make tests more reliable (#1808)

1.  Fix flaky behave tests with zookeeper. First, install/start binaries (zookeeper/localkube) and only after that continue with installing requirements and running behave. Previously zookeeper didn't had enough time to start and tests sometimes were failing.
2.  Fix flaky raft tests. Despite observations of MacOS slowness, for some unknown reason the delete test with a very small timeout was not timing out, but succeeding, causing unit-tests to fail. The solution - do not rely on the actual timeout, but mock it.
---
 .github/workflows/install_deps.py | 35 +++++++++++++++++--------------
 .github/workflows/tests.yaml      |  8 +++----
 tests/test_raft.py                |  8 ++++---
 3 files changed, 28 insertions(+), 23 deletions(-)

diff --git a/.github/workflows/install_deps.py b/.github/workflows/install_deps.py
index 1a942fe1..d3af36bf 100644
--- a/.github/workflows/install_deps.py
+++ b/.github/workflows/install_deps.py
@@ -44,7 +44,11 @@ def install_packages(what):
 
 
 def get_file(url, name):
-    from six.moves.urllib.request import urlretrieve
+    try:
+        from urllib.request import urlretrieve
+    except ImportError:
+        from urllib import urlretrieve
+
     print('Downloading ' + url)
     urlretrieve(url, name)
 
@@ -119,7 +123,6 @@ def setup_kubernetes():
                      stdout=devnull, stderr=devnull)
     for _ in range(0, 120):
         if subprocess.call(['wget', '-qO', '-', 'http://127.0.0.1:8080/'], stdout=devnull, stderr=devnull) == 0:
-            time.sleep(10)
             break
         time.sleep(1)
     else:
@@ -156,22 +159,22 @@ users:
 
 def main():
     what = os.environ.get('DCS', sys.argv[1] if len(sys.argv) > 1 else 'all')
-    r = install_requirements(what)
-    if what == 'all' or r != 0:
-        return r
 
-    if sys.platform.startswith('linux'):
-        r = install_packages(what)
-    else:
-        r = install_postgres()
-    if r != 0:
-        return r
+    if what != 'all':
+        if sys.platform.startswith('linux'):
+            r = install_packages(what)
+            if r == 0 and what == 'kubernetes':
+                r = setup_kubernetes()
+        else:
+            r = install_postgres()
 
-    if what.startswith('etcd'):
-        return install_etcd()
-    elif what == 'kubernetes':
-        return setup_kubernetes()
-    return 0
+        if r == 0 and what.startswith('etcd'):
+            r = install_etcd()
+
+        if r != 0:
+            return r
+
+    return install_requirements(what)
 
 
 if __name__ == '__main__':
diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml
index aaa19332..ccc5f7ed 100644
--- a/.github/workflows/tests.yaml
+++ b/.github/workflows/tests.yaml
@@ -86,7 +86,7 @@ jobs:
         COVERALLS_FLAG_NAME: unit-${{ matrix.os }}
         COVERALLS_PARALLEL: 'true'
         GITHUB_TOKEN: ${{ secrets.github_token }}
-      run: python -m coveralls
+      run: python -m coveralls --service=github
 
     - name: Run codacy-coverage-reporter
       uses: codacy/codacy-coverage-reporter-action@master
@@ -129,7 +129,7 @@ jobs:
         COVERALLS_FLAG_NAME: behave-${{ matrix.os }}-${{ matrix.dcs }}-${{ matrix.python-version }}
         COVERALLS_PARALLEL: 'true'
         GITHUB_TOKEN: ${{ secrets.github_token }}
-      run: python -m coveralls
+      run: python -m coveralls --service=github
 
   behavem:
     runs-on: ${{ matrix.os }}-latest
@@ -161,7 +161,7 @@ jobs:
         COVERALLS_FLAG_NAME: behave-${{ matrix.os }}-${{ matrix.dcs }}-${{ matrix.python-version }}
         COVERALLS_PARALLEL: 'true'
         GITHUB_TOKEN: ${{ secrets.github_token }}
-      run: python -m coveralls
+      run: python -m coveralls --service=github
 
   coveralls-finish:
     name: Finalize coveralls.io
@@ -170,6 +170,6 @@ jobs:
     steps:
     - uses: actions/setup-python@v2
     - run: python -m pip install coveralls
-    - run: python -m coveralls --finish
+    - run: python -m coveralls --service=github --finish
       env:
         GITHUB_TOKEN: ${{ secrets.github_token }}
diff --git a/tests/test_raft.py b/tests/test_raft.py
index cd79bfc4..d1e245bc 100644
--- a/tests/test_raft.py
+++ b/tests/test_raft.py
@@ -86,8 +86,6 @@ class TestKVStoreTTL(unittest.TestCase):
         self.so.set('foo', 'bar')
         self.so.set('fooo', 'bar')
         self.assertFalse(self.so.delete('foo', prevValue='buz'))
-        self.assertFalse(self.so.delete('foo', prevValue='bar', timeout=0.00001))
-        self.assertFalse(self.so.delete('foo', prevValue='bar'))
         self.assertTrue(self.so.delete('foo', recursive=True))
         self.assertFalse(self.so.retry(self.so._delete, 'foo', prevValue=''))
 
@@ -99,10 +97,14 @@ class TestKVStoreTTL(unittest.TestCase):
 
     @patch('time.sleep', Mock())
     def test_retry(self):
-        return_values = [FAIL_REASON.QUEUE_FULL, FAIL_REASON.SUCCESS, FAIL_REASON.REQUEST_DENIED]
+        return_values = [FAIL_REASON.QUEUE_FULL] * 2 + [FAIL_REASON.SUCCESS, FAIL_REASON.REQUEST_DENIED]
 
         def test(callback):
             callback(True, return_values.pop(0))
+
+        with patch('time.time', Mock(side_effect=[1, 100])):
+            self.assertFalse(self.so.retry(test))
+
         self.assertTrue(self.so.retry(test))
         self.assertFalse(self.so.retry(test))