Windows: fix flaky tests

Some tests compare timestamps (e.g. image created_at/updated_at
fields).

In some cases, subsequent timestamps may be identical, breaking the
assertions made by those tests.

One idea is to just add a 1ms sleep which should be a negligeable.
Mocking the functions used to retrieve timestamps may be undesireable.

For now, we'll do this only for Windows, where time.time has a lower
resolution compared to Linux (1e-7 as opposed to 1e-9).

At the same time, socket timeouts are rather inconsistent on Windows.
When a timeout is hit, recv may either throw ConnectionAbortedError
*or* return nothing. This needs to be taken into account when
expecting socket timeouts.

Change-Id: Ie5a4d8fb4c979c09eda2fdc0fad0baa1d1840c59
This commit is contained in:
Lucian Petrut 2019-03-14 15:46:50 +02:00
parent d501799a6a
commit 8fe78170e4
5 changed files with 44 additions and 16 deletions

View File

@ -17,7 +17,6 @@
import copy import copy
import datetime import datetime
import time
import uuid import uuid
import mock import mock
@ -139,6 +138,7 @@ class TestDriver(test_utils.BaseTestCase):
def create_images(self, images): def create_images(self, images):
for fixture in images: for fixture in images:
self.db_api.image_create(self.adm_context, fixture) self.db_api.image_create(self.adm_context, fixture)
self.delay_inaccurate_clock()
class DriverTests(object): class DriverTests(object):
@ -319,6 +319,7 @@ class DriverTests(object):
def test_image_update_properties(self): def test_image_update_properties(self):
fixture = {'properties': {'ping': 'pong'}} fixture = {'properties': {'ping': 'pong'}}
self.delay_inaccurate_clock()
image = self.db_api.image_update(self.adm_context, UUID1, fixture) image = self.db_api.image_update(self.adm_context, UUID1, fixture)
expected = {'ping': 'pong', 'foo': 'bar', 'far': 'boo'} expected = {'ping': 'pong', 'foo': 'bar', 'far': 'boo'}
actual = {p['name']: p['value'] for p in image['properties']} actual = {p['name']: p['value'] for p in image['properties']}
@ -1298,6 +1299,7 @@ class DriverTests(object):
'deleted': False} 'deleted': False}
self.assertEqual(expected, member) self.assertEqual(expected, member)
self.delay_inaccurate_clock()
member = self.db_api.image_member_update(self.context, member = self.db_api.image_member_update(self.context,
member_id, member_id,
{'can_share': True}) {'can_share': True})
@ -1341,10 +1343,7 @@ class DriverTests(object):
'deleted': False} 'deleted': False}
self.assertEqual(expected, member) self.assertEqual(expected, member)
# The clock may not be very accurate, for which reason we may self.delay_inaccurate_clock()
# get identical timestamps.
time.sleep(0.01)
member = self.db_api.image_member_update(self.context, member = self.db_api.image_member_update(self.context,
member_id, member_id,
{'status': 'accepted'}) {'status': 'accepted'})
@ -1872,6 +1871,7 @@ class TaskTests(test_utils.BaseTestCase):
'status': 'processing', 'status': 'processing',
'message': 'This is a error string', 'message': 'This is a error string',
} }
self.delay_inaccurate_clock()
task = self.db_api.task_update(self.adm_context, task_id, fixture) task = self.db_api.task_update(self.adm_context, task_id, fixture)
self.assertEqual(task_id, task['id']) self.assertEqual(task_id, task['id'])
@ -1897,6 +1897,7 @@ class TaskTests(test_utils.BaseTestCase):
task_id = task['id'] task_id = task['id']
fixture = {'status': 'processing'} fixture = {'status': 'processing'}
self.delay_inaccurate_clock()
task = self.db_api.task_update(self.adm_context, task_id, fixture) task = self.db_api.task_update(self.adm_context, task_id, fixture)
self.assertEqual(task_id, task['id']) self.assertEqual(task_id, task['id'])

View File

@ -44,18 +44,20 @@ class TestWSGIServer(testtools.TestCase):
port = server.sock.getsockname()[1] port = server.sock.getsockname()[1]
def get_request(delay=0.0): def get_request(delay=0.0):
# Socket timeouts are handled rather inconsistently on Windows.
# recv may either return nothing OR raise a ConnectionAbortedError.
exp_exc = OSError if os.name == 'nt' else ()
try:
sock = socket.socket() sock = socket.socket()
sock.connect(('127.0.0.1', port)) sock.connect(('127.0.0.1', port))
time.sleep(delay) time.sleep(delay)
sock.send(b'GET / HTTP/1.1\r\nHost: localhost\r\n\r\n') sock.send(b'GET / HTTP/1.1\r\nHost: localhost\r\n\r\n')
return sock.recv(1024) return sock.recv(1024)
except exp_exc:
return None
# Should succeed - no timeout # Should succeed - no timeout
self.assertIn(greetings, get_request()) self.assertIn(greetings, get_request())
# Should fail - connection timed out so we get nothing from the server # Should fail - connection timed out so we get nothing from the server
if os.name == 'nt':
self.assertRaises(ConnectionAbortedError,
get_request,
delay=1.1)
else:
self.assertFalse(get_request(delay=1.1)) self.assertFalse(get_request(delay=1.1))

View File

@ -377,6 +377,7 @@ class TestImageRepo(test_utils.BaseTestCase):
original_update_time = image.updated_at original_update_time = image.updated_at
image.name = 'foo' image.name = 'foo'
image.tags = ['king', 'kong'] image.tags = ['king', 'kong']
self.delay_inaccurate_clock()
self.image_repo.save(image) self.image_repo.save(image)
current_update_time = image.updated_at current_update_time = image.updated_at
self.assertGreater(current_update_time, original_update_time) self.assertGreater(current_update_time, original_update_time)
@ -396,6 +397,7 @@ class TestImageRepo(test_utils.BaseTestCase):
def test_remove_image(self): def test_remove_image(self):
image = self.image_repo.get(UUID1) image = self.image_repo.get(UUID1)
previous_update_time = image.updated_at previous_update_time = image.updated_at
self.delay_inaccurate_clock()
self.image_repo.remove(image) self.image_repo.remove(image)
self.assertGreater(image.updated_at, previous_update_time) self.assertGreater(image.updated_at, previous_update_time)
self.assertRaises(exception.ImageNotFound, self.image_repo.get, UUID1) self.assertRaises(exception.ImageNotFound, self.image_repo.get, UUID1)
@ -756,6 +758,7 @@ class TestTaskRepo(test_utils.BaseTestCase):
def test_save_task(self): def test_save_task(self):
task = self.task_repo.get(UUID1) task = self.task_repo.get(UUID1)
original_update_time = task.updated_at original_update_time = task.updated_at
self.delay_inaccurate_clock()
self.task_repo.save(task) self.task_repo.save(task)
current_update_time = task.updated_at current_update_time = task.updated_at
self.assertGreater(current_update_time, original_update_time) self.assertGreater(current_update_time, original_update_time)

View File

@ -135,6 +135,7 @@ class ImageCacheTestCase(object):
self.assertTrue(os.path.exists(incomplete_file_path)) self.assertTrue(os.path.exists(incomplete_file_path))
self.delay_inaccurate_clock()
self.cache.clean(stall_time=0) self.cache.clean(stall_time=0)
self.assertFalse(os.path.exists(incomplete_file_path)) self.assertFalse(os.path.exists(incomplete_file_path))

View File

@ -23,6 +23,7 @@ import shutil
import socket import socket
import subprocess import subprocess
import threading import threading
import time
from alembic import command as alembic_command from alembic import command as alembic_command
import fixtures import fixtures
@ -149,6 +150,26 @@ class BaseTestCase(testtools.TestCase):
self.addCleanup(patcher.stop) self.addCleanup(patcher.stop)
return result return result
def delay_inaccurate_clock(self, duration=0.001):
"""Add a small delay to compensate for inaccurate system clocks.
Some tests make assertions based on timestamps (e.g. comparing
'created_at' and 'updated_at' fields). In some cases, subsequent
time.time() calls may return identical values (python timestamps can
have a lower resolution on Windows compared to Linux - 1e-7 as
opposed to 1e-9).
A small delay (a few ms should be negligeable) can prevent such
issues. At the same time, it spares us from mocking the time
module, which might be undesired.
"""
# For now, we'll do this only for Windows. If really needed,
# on Py3 we can get the clock resolution using time.get_clock_info,
# but at that point we may as well just sleep 1ms all the time.
if os.name == 'nt':
time.sleep(duration)
class requires(object): class requires(object):
"""Decorator that initiates additional test setup/teardown.""" """Decorator that initiates additional test setup/teardown."""