From a32e568dde1804beebbbca30ce081e610f0d0c8f Mon Sep 17 00:00:00 2001 From: Gabriel Augendre Date: Sat, 29 Feb 2020 13:25:27 +0100 Subject: [PATCH 1/4] Refactor cli import in tests --- tests/test_end_to_end.py | 72 +++++++++++++--------------------------- 1 file changed, 23 insertions(+), 49 deletions(-) diff --git a/tests/test_end_to_end.py b/tests/test_end_to_end.py index a22cad0..61ee70c 100644 --- a/tests/test_end_to_end.py +++ b/tests/test_end_to_end.py @@ -39,17 +39,20 @@ class UtilsTestCase(unittest.TestCase): class ConfigTestCase(unittest.TestCase): - @mock.patch("click.edit") - @mock.patch("ofx_processor.utils.ynab.DEFAULT_CONFIG_DIR", "tests/samples") - def test_config_edit(self, edit): + def setUp(self): from ofx_processor.main import cli # This is run at import time and the cli module is already imported before this test # so we need to re-run the add_command to make it available. cli.add_command(ynab.config, name="config") + utils.discover_processors(cli) + self.cli = cli + @mock.patch("click.edit") + @mock.patch("ofx_processor.utils.ynab.DEFAULT_CONFIG_DIR", "tests/samples") + def test_config_edit(self, edit): runner = CliRunner() - runner.invoke(cli, ["config", "edit"]) + runner.invoke(self.cli, ["config", "edit"]) expected_filename = f"tests/samples/config.ini" edit.assert_called_once_with(filename=expected_filename) @@ -61,14 +64,8 @@ class ConfigTestCase(unittest.TestCase): ) @mock.patch("ofx_processor.utils.ynab.DEFAULT_CONFIG_DIR", "tests/samples") def test_broken_config_file(self, edit): - from ofx_processor.main import cli - - # This is run at import time and the cli module is already imported before this test - # so we need to re-run the add_command to make it available. - utils.discover_processors(cli) - runner = CliRunner() - result = runner.invoke(cli, ["revolut", "tests/samples/revolut.csv"]) + result = runner.invoke(self.cli, ["revolut", "tests/samples/revolut.csv"]) expected_filename = "tests/samples/config_broken_duplicate_key.ini" self.assertIn("Error while parsing config file", result.output) @@ -85,14 +82,9 @@ class ConfigTestCase(unittest.TestCase): @mock.patch("ofx_processor.utils.ynab.DEFAULT_CONFIG_FILENAME", "notfound.ini") @mock.patch("ofx_processor.utils.ynab.DEFAULT_CONFIG_DIR", "tests/samples") def test_missing_config_file(self, makedirs, edit): - from ofx_processor.main import cli - - # This is run at import time and the cli module is already imported before this test - # so we need to re-run the add_command to make it available. - utils.discover_processors(cli) runner = CliRunner() with mock.patch("ofx_processor.utils.ynab.open", mock.mock_open()) as mock_file: - result = runner.invoke(cli, ["revolut", "tests/samples/revolut.csv"]) + result = runner.invoke(self.cli, ["revolut", "tests/samples/revolut.csv"]) mock_file.assert_called_once() makedirs.assert_called_once_with(ynab.DEFAULT_CONFIG_DIR, exist_ok=True) self.assertIn("Editing config file", result.output) @@ -102,6 +94,15 @@ class ConfigTestCase(unittest.TestCase): @mock.patch("ofx_processor.utils.ynab.DEFAULT_CONFIG_DIR", "tests/samples") class DataTestCase(unittest.TestCase): + def setUp(self): + from ofx_processor.main import cli + + # This is run at import time and the cli module is already imported before this test + # so we need to re-run the add_command to make it available. + cli.add_command(ynab.config, name="config") + utils.discover_processors(cli) + self.cli = cli + @mock.patch("requests.post") def test_revolut_sends_data_only_created(self, post): post.return_value.json.return_value = { @@ -134,14 +135,9 @@ class DataTestCase(unittest.TestCase): "duplicate_import_ids": [], } } - from ofx_processor.main import cli - - # This is run at import time and the cli module is already imported before this test - # so we need to re-run the add_command to make it available. - utils.discover_processors(cli) runner = CliRunner() - result = runner.invoke(cli, ["revolut", "tests/samples/revolut.csv"]) + result = runner.invoke(self.cli, ["revolut", "tests/samples/revolut.csv"]) self.assertEqual(result.exit_code, 0) self.assertIn("Processed 9 transactions total.", result.output) @@ -181,14 +177,9 @@ class DataTestCase(unittest.TestCase): ], } } - from ofx_processor.main import cli - - # This is run at import time and the cli module is already imported before this test - # so we need to re-run the add_command to make it available. - utils.discover_processors(cli) runner = CliRunner() - result = runner.invoke(cli, ["revolut", "tests/samples/revolut.csv"]) + result = runner.invoke(self.cli, ["revolut", "tests/samples/revolut.csv"]) self.assertEqual(result.exit_code, 0) self.assertIn("Processed 9 transactions total.", result.output) @@ -213,14 +204,9 @@ class DataTestCase(unittest.TestCase): ], } } - from ofx_processor.main import cli - - # This is run at import time and the cli module is already imported before this test - # so we need to re-run the add_command to make it available. - utils.discover_processors(cli) runner = CliRunner() - result = runner.invoke(cli, ["revolut", "tests/samples/revolut.csv"]) + result = runner.invoke(self.cli, ["revolut", "tests/samples/revolut.csv"]) self.assertEqual(result.exit_code, 0) self.assertIn("Processed 9 transactions total.", result.output) @@ -229,12 +215,6 @@ class DataTestCase(unittest.TestCase): @mock.patch("requests.post") def test_bpvf_sends_to_ynab(self, post): - from ofx_processor.main import cli - - # This is run at import time and the cli module is already imported before this test - # so we need to re-run the add_command to make it available. - utils.discover_processors(cli) - with open("tests/samples/bpvf_transactions.json", encoding="utf-8") as f: expected_data = json.load(f) @@ -242,7 +222,7 @@ class DataTestCase(unittest.TestCase): expected_url = f"{ynab.BASE_URL}/budgets//transactions" runner = CliRunner() - runner.invoke(cli, ["bpvf", "tests/samples/bpvf.ofx"]) + runner.invoke(self.cli, ["bpvf", "tests/samples/bpvf.ofx"]) post.assert_called_once_with( expected_url, json=expected_data, headers=expected_headers @@ -250,12 +230,6 @@ class DataTestCase(unittest.TestCase): @mock.patch("requests.post") def test_ce_sends_to_ynab(self, post): - from ofx_processor.main import cli - - # This is run at import time and the cli module is already imported before this test - # so we need to re-run the add_command to make it available. - utils.discover_processors(cli) - with open("tests/samples/ce_transactions.json", encoding="utf-8") as f: expected_data = json.load(f) @@ -263,7 +237,7 @@ class DataTestCase(unittest.TestCase): expected_url = f"{ynab.BASE_URL}/budgets//transactions" runner = CliRunner() - runner.invoke(cli, ["ce", "tests/samples/ce.ofx"]) + runner.invoke(self.cli, ["ce", "tests/samples/ce.ofx"]) post.assert_called_once_with( expected_url, json=expected_data, headers=expected_headers From b1e80994b4d5c74e454c03e25b043dabf9d95f74 Mon Sep 17 00:00:00 2001 From: Gabriel Augendre Date: Sat, 29 Feb 2020 13:35:51 +0100 Subject: [PATCH 2/4] Improve and test bad config file handling --- ofx_processor/utils/ynab.py | 30 ++++++++++++------- .../samples/config_broken_missing_account.ini | 9 ++++++ .../config_broken_missing_account_key.ini | 11 +++++++ .../samples/config_broken_missing_budget.ini | 11 +++++++ tests/samples/config_broken_missing_token.ini | 11 +++++++ tests/test_end_to_end.py | 22 ++++++++++---- 6 files changed, 78 insertions(+), 16 deletions(-) create mode 100644 tests/samples/config_broken_missing_account.ini create mode 100644 tests/samples/config_broken_missing_account_key.ini create mode 100644 tests/samples/config_broken_missing_budget.ini create mode 100644 tests/samples/config_broken_missing_token.ini diff --git a/ofx_processor/utils/ynab.py b/ofx_processor/utils/ynab.py index 2b313c7..1e346cb 100644 --- a/ofx_processor/utils/ynab.py +++ b/ofx_processor/utils/ynab.py @@ -59,20 +59,21 @@ def push_transactions(transactions, account): try: config.read(config_file) except configparser.Error as e: - click.secho(f"Error while parsing config file: {str(e)}", fg="red", bold=True) - click.secho("Opening the file...") - click.pause() - click.edit(filename=config_file) - click.secho("Exiting...", fg="red", bold=True) - sys.exit(1) - section = config[account] - budget_id = section["budget"] + return handle_config_file_error(config_file, e) + + try: + section = config[account] + budget_id = section["budget"] + token = section["token"] + account = section["account"] + except KeyError as e: + return handle_config_file_error(config_file, e) + url = f"{BASE_URL}/budgets/{budget_id}/transactions" for transaction in transactions: - transaction["account_id"] = section["account"] + transaction["account_id"] = account data = {"transactions": transactions} - token = section["token"] headers = {"Authorization": f"Bearer {token}"} res = requests.post(url, json=data, headers=headers) @@ -95,3 +96,12 @@ def push_transactions(transactions, account): click.secho( f"{len(duplicates)} transactions ignored (duplicates).", fg="yellow" ) + + +def handle_config_file_error(config_file, e): + click.secho(f"Error while parsing config file: {str(e)}", fg="red", bold=True) + click.secho("Opening the file...") + click.pause() + click.edit(filename=config_file) + click.secho("Exiting...", fg="red", bold=True) + sys.exit(1) diff --git a/tests/samples/config_broken_missing_account.ini b/tests/samples/config_broken_missing_account.ini new file mode 100644 index 0000000..6f6bf8b --- /dev/null +++ b/tests/samples/config_broken_missing_account.ini @@ -0,0 +1,9 @@ +[DEFAULT] +token = +budget = + +[bpvf] +account = + +[ce] +account = diff --git a/tests/samples/config_broken_missing_account_key.ini b/tests/samples/config_broken_missing_account_key.ini new file mode 100644 index 0000000..04fc889 --- /dev/null +++ b/tests/samples/config_broken_missing_account_key.ini @@ -0,0 +1,11 @@ +[DEFAULT] +token = +budget = + +[bpvf] +account = + +[revolut] + +[ce] +account = diff --git a/tests/samples/config_broken_missing_budget.ini b/tests/samples/config_broken_missing_budget.ini new file mode 100644 index 0000000..90416f5 --- /dev/null +++ b/tests/samples/config_broken_missing_budget.ini @@ -0,0 +1,11 @@ +[DEFAULT] +token = + +[bpvf] +account = + +[revolut] +account = + +[ce] +account = diff --git a/tests/samples/config_broken_missing_token.ini b/tests/samples/config_broken_missing_token.ini new file mode 100644 index 0000000..cd7fb54 --- /dev/null +++ b/tests/samples/config_broken_missing_token.ini @@ -0,0 +1,11 @@ +[DEFAULT] +budget = + +[bpvf] +account = + +[revolut] +account = + +[ce] +account = diff --git a/tests/test_end_to_end.py b/tests/test_end_to_end.py index 61ee70c..c3d77e4 100644 --- a/tests/test_end_to_end.py +++ b/tests/test_end_to_end.py @@ -64,12 +64,22 @@ class ConfigTestCase(unittest.TestCase): ) @mock.patch("ofx_processor.utils.ynab.DEFAULT_CONFIG_DIR", "tests/samples") def test_broken_config_file(self, edit): - runner = CliRunner() - result = runner.invoke(self.cli, ["revolut", "tests/samples/revolut.csv"]) - - expected_filename = "tests/samples/config_broken_duplicate_key.ini" - self.assertIn("Error while parsing config file", result.output) - edit.assert_called_once_with(filename=expected_filename) + broken_files = [ + "config_broken_duplicate_key.ini", + "config_broken_missing_account.ini", + "config_broken_missing_account_key.ini", + "config_broken_missing_budget.ini", + "config_broken_missing_token.ini", + ] + for name in broken_files: + with mock.patch("ofx_processor.utils.ynab.DEFAULT_CONFIG_FILENAME", name): + runner = CliRunner() + result = runner.invoke( + self.cli, ["revolut", "tests/samples/revolut.csv"] + ) + expected_filename = ynab.get_config_file_name() + self.assertIn("Error while parsing config file", result.output) + edit.assert_called_with(filename=expected_filename) @mock.patch("ofx_processor.utils.ynab.DEFAULT_CONFIG_FILENAME", "file.ini") @mock.patch("ofx_processor.utils.ynab.DEFAULT_CONFIG_DIR", "some/config/folder") From 2cf722cd46daad30e613e3c11f293ba0bb2b9790 Mon Sep 17 00:00:00 2001 From: Gabriel Augendre Date: Sat, 29 Feb 2020 13:38:50 +0100 Subject: [PATCH 3/4] Improve sonar cloud feedback speed --- .github/workflows/package.yml | 49 ++++++++++++++++++----------------- sonar-project.properties | 2 +- 2 files changed, 26 insertions(+), 25 deletions(-) diff --git a/.github/workflows/package.yml b/.github/workflows/package.yml index f754379..81b75e2 100644 --- a/.github/workflows/package.yml +++ b/.github/workflows/package.yml @@ -6,6 +6,30 @@ on: types: [created] jobs: + sonarCloudTrigger: + name: SonarCloud Trigger + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - name: Set up Python + uses: actions/setup-python@v1.1.1 + with: + python-version: 3.8 + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install poetry + poetry install + - name: Test with pytest + run: | + poetry run coverage run --source=ofx_processor -m pytest --color=yes + poetry run coverage xml + - name: SonarCloud Scan + uses: sonarsource/sonarcloud-github-action@v1.1 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} + build: name: Python ${{ matrix.python-version }} / ${{ matrix.os }} runs-on: ${{ matrix.os }} @@ -29,30 +53,7 @@ jobs: poetry install - name: Test with pytest run: | - poetry run coverage run --source=ofx_processor -m pytest --color=yes - poetry run coverage xml - - name: Coverage report upload - uses: actions/upload-artifact@v1 - with: - name: coverage_${{ matrix.os }}_${{ matrix.python-version }} - path: coverage.xml - - sonarCloudTrigger: - name: SonarCloud Trigger - needs: build - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v2 - - name: Download coverage report - uses: actions/download-artifact@v1 - with: - name: coverage_ubuntu-latest_3.8 - path: coverage-reports - - name: SonarCloud Scan - uses: sonarsource/sonarcloud-github-action@v1.1 - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} + poetry run pytest --color=yes publish: name: Publish to PyPI diff --git a/sonar-project.properties b/sonar-project.properties index 54088c2..c6bb9be 100644 --- a/sonar-project.properties +++ b/sonar-project.properties @@ -4,4 +4,4 @@ sonar.projectKey=Crocmagnon_ofx-processor # relative paths to source directories. More details and properties are described # in https://sonarcloud.io/documentation/project-administration/narrowing-the-focus/ sonar.sources=ofx_processor -sonar.python.coverage.reportPaths=coverage-reports/coverage.xml +sonar.python.coverage.reportPaths=coverage.xml From 07ef54ad676bd52b18fa07b5d0c138611d9c17e7 Mon Sep 17 00:00:00 2001 From: Gabriel Augendre Date: Sat, 29 Feb 2020 13:42:33 +0100 Subject: [PATCH 4/4] Fix paths in tests on windows --- tests/test_end_to_end.py | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/tests/test_end_to_end.py b/tests/test_end_to_end.py index c3d77e4..ac53ddc 100644 --- a/tests/test_end_to_end.py +++ b/tests/test_end_to_end.py @@ -1,4 +1,5 @@ import json +import os import unittest from unittest import mock from unittest.mock import call @@ -49,12 +50,14 @@ class ConfigTestCase(unittest.TestCase): self.cli = cli @mock.patch("click.edit") - @mock.patch("ofx_processor.utils.ynab.DEFAULT_CONFIG_DIR", "tests/samples") + @mock.patch( + "ofx_processor.utils.ynab.DEFAULT_CONFIG_DIR", os.path.join("tests", "samples") + ) def test_config_edit(self, edit): runner = CliRunner() runner.invoke(self.cli, ["config", "edit"]) - expected_filename = f"tests/samples/config.ini" + expected_filename = os.path.join("tests", "samples", "config.ini") edit.assert_called_once_with(filename=expected_filename) @mock.patch("click.edit") @@ -62,7 +65,9 @@ class ConfigTestCase(unittest.TestCase): "ofx_processor.utils.ynab.DEFAULT_CONFIG_FILENAME", "config_broken_duplicate_key.ini", ) - @mock.patch("ofx_processor.utils.ynab.DEFAULT_CONFIG_DIR", "tests/samples") + @mock.patch( + "ofx_processor.utils.ynab.DEFAULT_CONFIG_DIR", os.path.join("tests", "samples") + ) def test_broken_config_file(self, edit): broken_files = [ "config_broken_duplicate_key.ini", @@ -82,15 +87,20 @@ class ConfigTestCase(unittest.TestCase): edit.assert_called_with(filename=expected_filename) @mock.patch("ofx_processor.utils.ynab.DEFAULT_CONFIG_FILENAME", "file.ini") - @mock.patch("ofx_processor.utils.ynab.DEFAULT_CONFIG_DIR", "some/config/folder") + @mock.patch( + "ofx_processor.utils.ynab.DEFAULT_CONFIG_DIR", + os.path.join("some", "config", "folder"), + ) def test_get_config_file_name(self): - expected = "some/config/folder/file.ini" + expected = os.path.join("some", "config", "folder", "file.ini") self.assertEqual(ynab.get_config_file_name(), expected) @mock.patch("click.edit") @mock.patch("os.makedirs") @mock.patch("ofx_processor.utils.ynab.DEFAULT_CONFIG_FILENAME", "notfound.ini") - @mock.patch("ofx_processor.utils.ynab.DEFAULT_CONFIG_DIR", "tests/samples") + @mock.patch( + "ofx_processor.utils.ynab.DEFAULT_CONFIG_DIR", os.path.join("tests", "samples") + ) def test_missing_config_file(self, makedirs, edit): runner = CliRunner() with mock.patch("ofx_processor.utils.ynab.open", mock.mock_open()) as mock_file: @@ -102,7 +112,9 @@ class ConfigTestCase(unittest.TestCase): edit.assert_called_once_with(filename=expected_filename) -@mock.patch("ofx_processor.utils.ynab.DEFAULT_CONFIG_DIR", "tests/samples") +@mock.patch( + "ofx_processor.utils.ynab.DEFAULT_CONFIG_DIR", os.path.join("tests", "samples") +) class DataTestCase(unittest.TestCase): def setUp(self): from ofx_processor.main import cli