From fda1eb3912fdc021ad8c2eb535311e40299d4800 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Boris=20Cl=C3=A9net?= <117362283+bclenet@users.noreply.github.com> Date: Fri, 29 Sep 2023 10:21:20 +0200 Subject: [PATCH] Fixing the `narps_open.utils.status` module (2) (#113) * [BUG] inside unit_tests workflow * Browsing all issues pages from Github API * Get all pages of GitHub issues * [TEST] Updating test for status module * [TEST] fetch several issues * Dealing with single page of issues --- narps_open/utils/status.py | 14 ++- tests/utils/test_status.py | 233 +++++++++++++++++++++++-------------- 2 files changed, 156 insertions(+), 91 deletions(-) diff --git a/narps_open/utils/status.py b/narps_open/utils/status.py index fef7708d..96d4265f 100644 --- a/narps_open/utils/status.py +++ b/narps_open/utils/status.py @@ -17,8 +17,16 @@ def get_opened_issues(): """ Return a list of opened issues and pull requests for the NARPS Open Pipelines project """ + + # First get the number of issues of the project + request_url = 'https://api.github.com/repos/Inria-Empenn/narps_open_pipelines' + response = get(request_url, timeout = 2) + response.raise_for_status() + nb_issues = response.json()['open_issues'] + + # Get all opened issues request_url = 'https://api.github.com/repos/Inria-Empenn/narps_open_pipelines/issues' - request_url += '?page={page_number}?per_page=100' + request_url += '?page={page_number}?per_page=30' issues = [] page = True # Will later be replaced by a table @@ -31,6 +39,10 @@ def get_opened_issues(): issues += page page_number += 1 + # Leave if there is only one page (in this case, the `page` query parameter has no effect) + if nb_issues < 30: + break + return issues def get_teams_with_pipeline_files(): diff --git a/tests/utils/test_status.py b/tests/utils/test_status.py index 6c16279b..42f9f584 100644 --- a/tests/utils/test_status.py +++ b/tests/utils/test_status.py @@ -25,6 +25,118 @@ PipelineStatusReport ) +mocked_issues_4 = [ + { + "html_url": "url_issue_2", + "number": 2, + "title" : "Issue for pipeline UK24", + "body" : "Nothing to add here." + }, + { + "html_url": "url_pull_3", + "number": 3, + "title" : "Pull request for pipeline 2T6S", + "pull_request" : {}, + "body" : "Work has been done." + }, + { + "html_url": "url_issue_4", + "number": 4, + "title" : None, + "body" : "This is a malformed issue about C88N." + }, + { + "html_url": "url_issue_5", + "number": 5, + "title" : "Issue about 2T6S", + "body" : "Something about 2T6S." + } +] + +mocked_issues_40_1 = [ + { + "html_url": "url_issue_55", + "number": i, + "title" : "Issue for pipeline UK24", + "body" : "Nothing to add here." + } for i in range(0,30) +] + +mocked_issues_40_2 = [ + { + "html_url": "url_issue_55", + "number": i, + "title" : "Issue for pipeline UK24", + "body" : "Nothing to add here." + } for i in range(0,10) +] + +mocked_repo_info_0 = { + "open_issues": 0 +} + +mocked_repo_info_4 = { + "open_issues": 4 +} + +mocked_repo_info_40 = { + "open_issues": 40 +} + +def mocked_requests_get_0(url, params=None, **kwargs): + """ Create a method to mock requests.get in case there are no issues """ + + response = Response() + response.status_code = 200 + + if 'issues' not in url: + def mocked_json(): + return mocked_repo_info_0 + else: + def mocked_json(): + return [] + + response.json = mocked_json + return response + +def mocked_requests_get_4(url, params=None, **kwargs): + """ Create a method to mock requests.get in case there are less than 30 issues """ + + response = Response() + response.status_code = 200 + + if 'issues' not in url: + def mocked_json(): + return mocked_repo_info_4 + else: + def mocked_json(): + return mocked_issues_4 + + response.json = mocked_json + return response + +def mocked_requests_get_40(url, params=None, **kwargs): + """ Create a method to mock requests.get in case there are more than 30 issues """ + + response = Response() + response.status_code = 200 + + if 'issues' not in url: + def mocked_json(): + return mocked_repo_info_40 + elif '?page=1' in url: + def mocked_json(): + return mocked_issues_40_1 + elif '?page=2' in url: + def mocked_json(): + return mocked_issues_40_2 + else: + def mocked_json(): + return [] + + response.json = mocked_json + return response + @fixture def mock_api_issue(mocker): """ Create a mock GitHub API response for successful query on open issues @@ -35,61 +147,6 @@ def mock_api_issue(mocker): Hence, we patch the `narps_open.utils.status.get` method. """ - # Create a method to mock requests.get - def mocked_requests_get(url, params=None, **kwargs): - - response = Response() - response.status_code = 200 - def json_func_page_0(): - return [ - { - "html_url": "url_issue_2", - "number": 2, - "title" : "Issue for pipeline UK24", - "body" : "Nothing to add here." - }, - { - "html_url": "url_pull_3", - "number": 3, - "title" : "Pull request for pipeline 2T6S", - "pull_request" : {}, - "body" : "Work has been done." - } - ] - - json_func_page_1 = json_func_page_0 - - def json_func_page_2(): - return [ - { - "html_url": "url_issue_4", - "number": 4, - "title" : None, - "body" : "This is a malformed issue about C88N." - }, - { - "html_url": "url_issue_5", - "number": 5, - "title" : "Issue about 2T6S", - "body" : "Something about 2T6S." - } - ] - - def json_func_page_3(): - return [] - - if '?page=1' in url: - response.json = json_func_page_1 - elif '?page=2' in url: - response.json = json_func_page_2 - elif '?page=3' in url: - response.json = json_func_page_3 - else: - response.json = json_func_page_0 - - return response - - mocker.patch('narps_open.utils.status.get', side_effect = mocked_requests_get) mocker.patch( 'narps_open.utils.status.get_teams_with_pipeline_files', return_value = ['2T6S', 'UK24', 'Q6O0'] @@ -118,7 +175,7 @@ def test_get_issues(mocker): which is actually imported as `get` inside the `narps_open.utils.status` module. Hence, we patch the `narps_open.utils.status.get` method. """ - # Create a mock API response for 404 error + # Behavior for 404 error response = Response() response.status_code = 404 @@ -126,41 +183,28 @@ def test_get_issues(mocker): with raises(HTTPError): get_opened_issues() - # Create a mock API response for the no issues case - response = Response() - response.status_code = 200 - def json_func(): - return [] - response.json = json_func - mocker.patch('narps_open.utils.status.get', return_value = response) + # No issues case + mocker.patch('narps_open.utils.status.get', side_effect = mocked_requests_get_0) assert len(get_opened_issues()) == 0 - # Create a mock API response for the general usecase - def mocked_requests_get(url, params=None, **kwargs): - response = Response() - response.status_code = 200 - def json_func_page_1(): - return [ - { - "html_url": "urls", - "number": 2, - } - ] - def json_func_page_2(): - return [] - - if '?page=2' in url: - response.json = json_func_page_2 - else: - response.json = json_func_page_1 - - return response - - mocker.patch('narps_open.utils.status.get', side_effect = mocked_requests_get) + # General usecase 4 issues + mocker.patch('narps_open.utils.status.get', side_effect = mocked_requests_get_4) + issues = get_opened_issues() - assert len(issues) == 1 - assert issues[0]['html_url'] == 'urls' + assert len(issues) == 4 + assert issues[0]['html_url'] == 'url_issue_2' assert issues[0]['number'] == 2 + assert issues[0]['title'] == 'Issue for pipeline UK24' + assert issues[0]['body'] == 'Nothing to add here.' + + # General usecase 40 issues + mocker.patch('narps_open.utils.status.get', side_effect = mocked_requests_get_40) + issues = get_opened_issues() + assert len(issues) == 40 + assert issues[0]['html_url'] == 'url_issue_55' + assert issues[0]['number'] == 0 + assert issues[0]['title'] == 'Issue for pipeline UK24' + assert issues[0]['body'] == 'Nothing to add here.' @staticmethod @mark.unit_test @@ -175,9 +219,12 @@ def test_get_teams(): @staticmethod @mark.unit_test - def test_generate(mock_api_issue): + def test_generate(mock_api_issue, mocker): """ Test generating a PipelineStatusReport """ + # Mock requests.get + mocker.patch('narps_open.utils.status.get', side_effect = mocked_requests_get_4) + # Test the generation report = PipelineStatusReport() report.generate() @@ -223,9 +270,12 @@ def test_generate(mock_api_issue): @staticmethod @mark.unit_test - def test_markdown(mock_api_issue): + def test_markdown(mock_api_issue, mocker): """ Test writing a PipelineStatusReport as Markdown """ + # Mock requests.get + mocker.patch('narps_open.utils.status.get', side_effect = mocked_requests_get_4) + # Generate markdown from report report = PipelineStatusReport() report.generate() @@ -241,9 +291,12 @@ def test_markdown(mock_api_issue): @staticmethod @mark.unit_test - def test_str(mock_api_issue): + def test_str(mock_api_issue, mocker): """ Test writing a PipelineStatusReport as JSON """ + # Mock requests.get + mocker.patch('narps_open.utils.status.get', side_effect = mocked_requests_get_4) + # Generate report report = PipelineStatusReport() report.generate()