From 2790bc17dd906119db524b1827308150be57bbd7 Mon Sep 17 00:00:00 2001 From: meruyert93 <61665812+meruyert93@users.noreply.github.com> Date: Thu, 2 May 2024 14:43:55 +0200 Subject: [PATCH 1/5] Fix polling duration calculation in poll_with_specified_overhead function --- railib/api.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/railib/api.py b/railib/api.py index daeb93f..c1c1e5b 100644 --- a/railib/api.py +++ b/railib/api.py @@ -333,6 +333,7 @@ def poll_with_specified_overhead( ): if start_time is None: start_time = time.time() + tries = 0 max_time = time.time() + timeout if timeout else None @@ -340,18 +341,19 @@ def poll_with_specified_overhead( if f(): break + current_time = time.time() + if max_tries is not None and tries >= max_tries: raise Exception(f'max tries {max_tries} exhausted') - if max_time is not None and time.time() >= max_time: + if max_time is not None and current_time >= max_time: raise Exception(f'timed out after {timeout} seconds') + duration = (current_time - start_time) * overhead_rate + duration = min(duration, max_delay) + + time.sleep(duration) tries += 1 - duration = min((time.time() - start_time) * overhead_rate, max_delay) - if tries == 1: - time.sleep(0.5) - else: - time.sleep(duration) def is_engine_term_state(state: str) -> bool: From a2621fe1c56549ba01d0940e3a3b2267ab3ed7d7 Mon Sep 17 00:00:00 2001 From: meruyert93 <61665812+meruyert93@users.noreply.github.com> Date: Fri, 3 May 2024 14:05:46 +0200 Subject: [PATCH 2/5] add more tests --- test/test_unit.py | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/test/test_unit.py b/test/test_unit.py index d5a4e7b..c14ab93 100644 --- a/test/test_unit.py +++ b/test/test_unit.py @@ -1,4 +1,5 @@ import socket +import time import unittest from unittest.mock import patch, MagicMock from urllib.error import URLError @@ -27,6 +28,42 @@ def test_validation(self): api.poll_with_specified_overhead(lambda: True, overhead_rate=0.1, max_tries=1) api.poll_with_specified_overhead(lambda: True, overhead_rate=0.1, timeout=1, max_tries=1) + def test_initial_delay(self): + start_time = time.time() + with patch('time.sleep') as mock_sleep: + try: + api.poll_with_specified_overhead(lambda: False, overhead_rate=0.1, max_tries=1) + except Exception: + pass + mock_sleep.assert_called_with((time.time() - start_time) * 0.1) + + def test_max_delay_cap(self): + with patch('time.sleep') as mock_sleep: + try: + api.poll_with_specified_overhead(lambda: False, overhead_rate=1, max_tries=2, start_time=time.time() - 200) + except Exception: + pass + # Ensure that the maximum delay of 120 seconds is not exceeded + mock_sleep.assert_called_with(120) + + def test_polling_success(self): + with patch('time.sleep', return_value=None) as mock_sleep: + api.poll_with_specified_overhead(lambda: True, overhead_rate=0.1) + mock_sleep.assert_not_called() + + def test_negative_overhead_rate(self): + with self.assertRaises(ValueError): + api.poll_with_specified_overhead(lambda: True, overhead_rate=-0.1) + + def test_realistic_scenario(self): + responses = [False, False, True] + def side_effect(): + return responses.pop(0) + + with patch('time.sleep') as mock_sleep: + api.poll_with_specified_overhead(side_effect, overhead_rate=0.1, max_tries=3) + # Ensure that `time.sleep` was called twice (for two false returns) + self.assertEqual(mock_sleep.call_count, 2) @patch('railib.rest.urlopen') class TestURLOpenWithRetry(unittest.TestCase): From 741e022adff3fe6cfa9242c19320ba6c56319568 Mon Sep 17 00:00:00 2001 From: meruyert93 <61665812+meruyert93@users.noreply.github.com> Date: Fri, 3 May 2024 14:21:11 +0200 Subject: [PATCH 3/5] fix test --- railib/api.py | 3 +++ test/test_unit.py | 18 ++++++++++-------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/railib/api.py b/railib/api.py index c1c1e5b..69eeebe 100644 --- a/railib/api.py +++ b/railib/api.py @@ -331,6 +331,9 @@ def poll_with_specified_overhead( max_tries: int = None, max_delay: int = 120, ): + if overhead_rate < 0: + raise ValueError("overhead_rate must be non-negative") + if start_time is None: start_time = time.time() diff --git a/test/test_unit.py b/test/test_unit.py index c14ab93..bdd9574 100644 --- a/test/test_unit.py +++ b/test/test_unit.py @@ -10,6 +10,8 @@ class TestPolling(unittest.TestCase): + @patch('time.sleep', return_value=None) + @patch('time.time') def test_timeout_exception(self): try: api.poll_with_specified_overhead(lambda: False, overhead_rate=0.1, timeout=1) @@ -28,14 +30,14 @@ def test_validation(self): api.poll_with_specified_overhead(lambda: True, overhead_rate=0.1, max_tries=1) api.poll_with_specified_overhead(lambda: True, overhead_rate=0.1, timeout=1, max_tries=1) - def test_initial_delay(self): - start_time = time.time() - with patch('time.sleep') as mock_sleep: - try: - api.poll_with_specified_overhead(lambda: False, overhead_rate=0.1, max_tries=1) - except Exception: - pass - mock_sleep.assert_called_with((time.time() - start_time) * 0.1) + def test_initial_delay(self, mock_time, mock_sleep): + start_time = 100 # Fixed start time + mock_time.return_value = start_time + 0.0001 # Fixed increment + + api.poll_with_specified_overhead(lambda: False, overhead_rate=0.1, max_tries=1) + + expected_sleep_time = (mock_time.return_value - start_time) * 0.1 + mock_sleep.assert_called_with(expected_sleep_time) def test_max_delay_cap(self): with patch('time.sleep') as mock_sleep: From c309d9f36dfe03c29b938900645fdcc94697cbc9 Mon Sep 17 00:00:00 2001 From: meruyert93 <61665812+meruyert93@users.noreply.github.com> Date: Fri, 3 May 2024 14:34:35 +0200 Subject: [PATCH 4/5] move decorator to the specific test --- .github/actions/test/action.yml | 2 +- test/test_unit.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/actions/test/action.yml b/.github/actions/test/action.yml index fa4f60e..d8ac946 100644 --- a/.github/actions/test/action.yml +++ b/.github/actions/test/action.yml @@ -61,5 +61,5 @@ runs: CUSTOM_HEADERS: ${{ inputs.custom_headers }} run: | mkdir -p ~/.rai - python -m unittest + python -m unittest -v shell: bash diff --git a/test/test_unit.py b/test/test_unit.py index bdd9574..41ba353 100644 --- a/test/test_unit.py +++ b/test/test_unit.py @@ -10,8 +10,6 @@ class TestPolling(unittest.TestCase): - @patch('time.sleep', return_value=None) - @patch('time.time') def test_timeout_exception(self): try: api.poll_with_specified_overhead(lambda: False, overhead_rate=0.1, timeout=1) @@ -30,6 +28,8 @@ def test_validation(self): api.poll_with_specified_overhead(lambda: True, overhead_rate=0.1, max_tries=1) api.poll_with_specified_overhead(lambda: True, overhead_rate=0.1, timeout=1, max_tries=1) + @patch('time.sleep', return_value=None) + @patch('time.time') def test_initial_delay(self, mock_time, mock_sleep): start_time = 100 # Fixed start time mock_time.return_value = start_time + 0.0001 # Fixed increment From d64e9f49ea57d77473c128574bc2ad4b57e7fae3 Mon Sep 17 00:00:00 2001 From: meruyert93 <61665812+meruyert93@users.noreply.github.com> Date: Fri, 3 May 2024 14:44:48 +0200 Subject: [PATCH 5/5] fix test --- test/test_unit.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/test/test_unit.py b/test/test_unit.py index 41ba353..27b8665 100644 --- a/test/test_unit.py +++ b/test/test_unit.py @@ -32,11 +32,15 @@ def test_validation(self): @patch('time.time') def test_initial_delay(self, mock_time, mock_sleep): start_time = 100 # Fixed start time - mock_time.return_value = start_time + 0.0001 # Fixed increment + increment_time = 0.0001 + mock_time.side_effect = [start_time, start_time + increment_time] # Simulate time progression - api.poll_with_specified_overhead(lambda: False, overhead_rate=0.1, max_tries=1) + try: + api.poll_with_specified_overhead(lambda: False, overhead_rate=0.1, max_tries=2) + except Exception as e: + pass # Ignore the exception for this test - expected_sleep_time = (mock_time.return_value - start_time) * 0.1 + expected_sleep_time = (start_time + increment_time - start_time) * 0.1 mock_sleep.assert_called_with(expected_sleep_time) def test_max_delay_cap(self):