From 6986e90351bca4071ea61387bee1f52f1125aceb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Boris=20Cl=C3=A9net?= Date: Thu, 31 Aug 2023 14:35:10 +0200 Subject: [PATCH 1/6] [BUG] inside unit_tests workflow --- .github/workflows/unit_tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/unit_tests.yml b/.github/workflows/unit_tests.yml index 20f20ea3..d0097882 100644 --- a/.github/workflows/unit_tests.yml +++ b/.github/workflows/unit_tests.yml @@ -34,7 +34,7 @@ jobs: - name: Checkout repository uses: actions/checkout@v3 - - name: Load configuration for self-hosted runner + - name: Load configuration for self-hosted runner run: cp /home/neuro/local_testing_config.toml narps_open/utils/configuration/testing_config.toml - name: Install dependencies From 2cc3c8048e92bd2c054a0a35cc7d337b34803eee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Boris=20Cl=C3=A9net?= Date: Fri, 22 Sep 2023 17:33:02 +0200 Subject: [PATCH 2/6] Browsing all issues pages from Github API --- narps_open/utils/status.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/narps_open/utils/status.py b/narps_open/utils/status.py index cc4eb8a7..333a046e 100644 --- a/narps_open/utils/status.py +++ b/narps_open/utils/status.py @@ -18,10 +18,18 @@ def get_opened_issues(): """ Return a list of opened issues and pull requests for the NARPS Open Pipelines project """ request_url = 'https://api.github.com/repos/Inria-Empenn/narps_open_pipelines/issues' - response = get(request_url, timeout = 2) - response.raise_for_status() - - return response.json() + request_url += '?page={page_number}' + + issues = {} + page = True # Will later be replaced by a dict + page_number = 0 + while bool(page) : + response = get(request_url.format(page_number = str(page_number)), timeout = 2) + response.raise_for_status() + page = response.json() + issues += page + + return issues def get_teams_with_pipeline_files(): """ Return a set of teams having a file for their pipeline in the repository """ From 01845d9df9b50b5011b198797916e3e81b0d9b54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Boris=20Cl=C3=A9net?= Date: Mon, 25 Sep 2023 11:26:37 +0200 Subject: [PATCH 3/6] Get all pages of GitHub issues --- narps_open/utils/status.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/narps_open/utils/status.py b/narps_open/utils/status.py index 333a046e..e70e984d 100644 --- a/narps_open/utils/status.py +++ b/narps_open/utils/status.py @@ -20,10 +20,11 @@ def get_opened_issues(): request_url = 'https://api.github.com/repos/Inria-Empenn/narps_open_pipelines/issues' request_url += '?page={page_number}' - issues = {} + issues = [] page = True # Will later be replaced by a dict - page_number = 0 - while bool(page) : + page_number = 1 # According to the doc, first page is not page 0 + # https://docs.github.com/en/rest/issues/issues#list-repository-issues + while page : # TODO check if page is empty response = get(request_url.format(page_number = str(page_number)), timeout = 2) response.raise_for_status() page = response.json() From 6a8472cfe20ddee796590daf3ad1b7ed8f3a5545 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Boris=20Cl=C3=A9net?= Date: Mon, 25 Sep 2023 14:34:31 +0200 Subject: [PATCH 4/6] [TEST] Updating test for status module --- docs/status.md | 2 +- narps_open/utils/status.py | 4 ++-- tests/utils/test_status.py | 14 ++++++++++++-- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/docs/status.md b/docs/status.md index f323cc8f..7fde8239 100644 --- a/docs/status.md +++ b/docs/status.md @@ -36,7 +36,7 @@ print(pipeline_info['status']) report.markdown() # Returns a string containing the markdown ``` -You can also use the command-line tool as so. Option `-t` is for the team id, option `-d` allows to print only one of the sub parts of the description among : `general`, `exclusions`, `preprocessing`, `analysis`, and `categorized_for_analysis`. +You can also use the command-line tool as so. ```bash python narps_open/utils/status -h diff --git a/narps_open/utils/status.py b/narps_open/utils/status.py index e70e984d..e919b0fc 100644 --- a/narps_open/utils/status.py +++ b/narps_open/utils/status.py @@ -21,10 +21,10 @@ def get_opened_issues(): request_url += '?page={page_number}' issues = [] - page = True # Will later be replaced by a dict + page = True # Will later be replaced by a table page_number = 1 # According to the doc, first page is not page 0 # https://docs.github.com/en/rest/issues/issues#list-repository-issues - while page : # TODO check if page is empty + while bool(page) is True : # Test if the page is empty response = get(request_url.format(page_number = str(page_number)), timeout = 2) response.raise_for_status() page = response.json() diff --git a/tests/utils/test_status.py b/tests/utils/test_status.py index 0e98ef83..49c13a6f 100644 --- a/tests/utils/test_status.py +++ b/tests/utils/test_status.py @@ -36,7 +36,7 @@ def mock_api_issue(mocker): """ response = Response() response.status_code = 200 - def json_func(): + def json_func_page_0(): return [ { "html_url": "url_issue_2", @@ -50,7 +50,13 @@ def json_func(): "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, @@ -64,6 +70,10 @@ def json_func(): "body" : "Something about 2T6S." } ] + + def json_func_page_3(): + return [] + response.json = json_func mocker.patch('narps_open.utils.status.get', return_value = response) mocker.patch( From e18ea00a378335217dec12b07b637b5dcd6e0e23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Boris=20Cl=C3=A9net?= Date: Tue, 26 Sep 2023 12:03:46 +0200 Subject: [PATCH 5/6] [TEST] fetch several issues --- narps_open/utils/status.py | 3 +- tests/utils/test_status.py | 133 +++++++++++++++++++++---------------- 2 files changed, 79 insertions(+), 57 deletions(-) diff --git a/narps_open/utils/status.py b/narps_open/utils/status.py index e919b0fc..fef7708d 100644 --- a/narps_open/utils/status.py +++ b/narps_open/utils/status.py @@ -18,7 +18,7 @@ def get_opened_issues(): """ Return a list of opened issues and pull requests for the NARPS Open Pipelines project """ request_url = 'https://api.github.com/repos/Inria-Empenn/narps_open_pipelines/issues' - request_url += '?page={page_number}' + request_url += '?page={page_number}?per_page=100' issues = [] page = True # Will later be replaced by a table @@ -29,6 +29,7 @@ def get_opened_issues(): response.raise_for_status() page = response.json() issues += page + page_number += 1 return issues diff --git a/tests/utils/test_status.py b/tests/utils/test_status.py index 49c13a6f..6c16279b 100644 --- a/tests/utils/test_status.py +++ b/tests/utils/test_status.py @@ -34,48 +34,62 @@ def mock_api_issue(mocker): which is actually imported as `get` inside the `narps_open.utils.status` module. Hence, we patch the `narps_open.utils.status.get` method. """ - 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 [] - - response.json = json_func - mocker.patch('narps_open.utils.status.get', return_value = response) + + # 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'] @@ -104,8 +118,6 @@ 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. """ - get_opened_issues() - # Create a mock API response for 404 error response = Response() response.status_code = 404 @@ -124,18 +136,27 @@ def json_func(): assert len(get_opened_issues()) == 0 # Create a mock API response for the general usecase - response = Response() - response.status_code = 200 - def json_func(): - return [ - { - "html_url": "urls", - "number": 2, - } - ] - response.json = json_func - - mocker.patch('narps_open.utils.status.get', return_value = response) + 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) issues = get_opened_issues() assert len(issues) == 1 assert issues[0]['html_url'] == 'urls' From f9f7a550dc509c690f92c967aee0b6093a2d61ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Boris=20Cl=C3=A9net?= Date: Fri, 29 Sep 2023 10:10:46 +0200 Subject: [PATCH 6/6] Dealing with single page of issues --- narps_open/utils/status.py | 13 ++- tests/utils/test_status.py | 233 ++++++++++++++++++++++--------------- 2 files changed, 154 insertions(+), 92 deletions(-) diff --git a/narps_open/utils/status.py b/narps_open/utils/status.py index fef7708d..56b64052 100644 --- a/narps_open/utils/status.py +++ b/narps_open/utils/status.py @@ -17,8 +17,15 @@ 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 +38,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..41143bbf 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 @@ -34,62 +146,6 @@ def mock_api_issue(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 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 +174,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 +182,27 @@ 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 +217,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 +268,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 +289,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()