apply suggestions from code review

Author: umbynos

.flake8

@@ -6,4 +6,4 @@ ignore =
     W503
 max-complexity = 10
 max-line-length = 120
-select = E,W,F,C,N
+select = E,W,F,C,N

.github/workflows/test.yaml

@@ -32,6 +32,10 @@ jobs:
           repo-token: ${{ secrets.GITHUB_TOKEN }}
           version: 3.x
 
+      - name: Build native
+        shell: bash
+        run: task build
+
       - name: Cross-build for 386
         if: matrix.os != 'macos-latest'
         env:
@@ -44,10 +48,6 @@ jobs:
           GOARCH: "arm"
         run: task build
 
-      - name: Build native
-        shell: bash
-        run: task build
-
       - name: Install Python
         uses: actions/setup-python@v2
         with:

Taskfile.yml

@@ -45,6 +45,7 @@ tasks:
   test-integration:
     desc: Run integration tests
     cmds:
+      - task: build
       - poetry install --no-root
       - poetry run pytest test
 

poetry.lock

@@ -9,11 +9,12 @@ python = "^3.8"
 pytest = "6.1.2"
 invoke = "^1.5.0"
 semver = "^2.13.0"
+python-dateutil = "^2.8.1"
 
 [tool.poetry.dev-dependencies]
 flake8 = "^3.9.2"
 black = "^21.5b1"
 
 [tool.black]
 line-length = 120
-target-version = ["py38"]
+target-version = ["py38"]

pyproject.toml

@@ -17,23 +17,32 @@
 
 import pathlib
 import platform
+import typing
 import invoke.context
 import pytest
 
 
 @pytest.fixture(scope="function")
-def run_command(pytestconfig, working_dir):
+def run_command(pytestconfig, working_dir) -> typing.Callable[..., invoke.runners.Result]:
     """Provide a wrapper around invoke's `run` API so that every test will work in the same temporary folder.
+
     Useful reference:
         http://docs.pyinvoke.org/en/1.4/api/runners.html#invoke.runners.Result
     """
 
     fwuploader_path = pathlib.Path(pytestconfig.rootdir).parent / "FirmwareUploader"
 
-    def _run(cmd_string, custom_working_dir=None, custom_env=None):
+    def _run(
+        cmd: list, custom_working_dir: typing.Optional[str] = None, custom_env: typing.Optional[dict] = None
+    ) -> invoke.runners.Result:
+        if cmd is None:
+            cmd = []
         if not custom_working_dir:
             custom_working_dir = working_dir
-        cli_full_line = '"{}" {}'.format(fwuploader_path, cmd_string)
+        quoted_cmd = []
+        for token in cmd:
+            quoted_cmd.append(f'"{token}"')
+        cli_full_line = '"{}" {}'.format(fwuploader_path, " ".join(quoted_cmd))
         run_context = invoke.context.Context()
         # It might happen that we need to change directories between drives on Windows,
         # in that case the "/d" flag must be used otherwise directory wouldn't change

test/conftest.py

@@ -6,4 +6,4 @@ filterwarnings =
 
 # --capture=no - disable per-test capture
 # --tb=long sets the length of the traceback in case of failures
-addopts = --capture=no --tb=long --verbose
+addopts = --capture=no --tb=long --verbose

test/pytest.ini

@@ -17,28 +17,30 @@
 
 import json
 import semver
-
-
-def test_help(run_command):
-    result = run_command("help")
-    assert result.ok
-    assert result.stderr == ""
-    assert "Usage" in result.stdout
+import dateutil.parser
 
 
 def test_version(run_command):
-    result = run_command("version")
+    result = run_command(cmd=["version"])
     assert result.ok
-    assert "Version:" in result.stdout
-    assert "Commit:" in result.stdout
-    assert "Date:" in result.stdout
+    output_list = result.stdout.strip().split(sep=" ")
+    assert output_list[0] == "FirmwareUploader"
+    assert output_list[1] == "Version:"
+    version = output_list[2]
+    assert semver.VersionInfo.isvalid(version=version) or version == "git-snapshot" or "nightly" in version
+    assert output_list[3] == "Commit:"
+    assert isinstance(output_list[4], str)
+    assert output_list[5] == "Date:"
+    assert dateutil.parser.isoparse(output_list[6])
     assert "" == result.stderr
 
-    result = run_command("version --format json")
+    result = run_command(cmd=["version", "--format", "json"])
     assert result.ok
     parsed_out = json.loads(result.stdout)
     assert parsed_out.get("Application", False) == "FirmwareUploader"
     version = parsed_out.get("VersionString", False)
     assert semver.VersionInfo.isvalid(version=version) or "git-snapshot" in version or "nightly" in version
+    assert parsed_out.get("Commit", False) != ""
     assert isinstance(parsed_out.get("Commit", False), str)
+    assert parsed_out.get("Date") != ""
     assert isinstance(parsed_out.get("Date", False), str)