Make CI jobs use matrix.os value and fix tests for macOS & Windows (#211)

* Make CI jobs use matrix.os value and fix config

Signed-off-by: Christophe Bedard <[email protected]>

* Do macOS-specific setup

Signed-off-by: Christophe Bedard <[email protected]>

* Disable Windows CI job

Signed-off-by: Christophe Bedard <[email protected]>

* Re-enable Windows job

Signed-off-by: Christophe Bedard <[email protected]>

* Fix tests on Windows

Signed-off-by: Christophe Bedard <[email protected]>

* reorder functions to minimize diff

* reduce logic change, keep comparing strings, not bytes

Co-authored-by: Dirk Thomas <[email protected]>

Author: christophebedard

.github/workflows/ci.yml

@@ -8,7 +8,7 @@ on:
 jobs:
   build:
 
-    runs-on: ubuntu-latest
+    runs-on: ${{ matrix.os }}
     strategy:
       matrix:
         os: [ubuntu-latest]
@@ -29,8 +29,13 @@ jobs:
       run: |
         python -m pip install --upgrade pip setuptools wheel
         python -m pip install --upgrade PyYAML
+    - name: Install dependencies (macOS)
+      run: |
+        brew install subversion mercurial
+      if: matrix.os == 'macos-latest'
     - name: Test with pytest
       run: |
         pip install --upgrade coverage flake8 flake8-docstrings flake8-import-order pytest
         git config --global --add init.defaultBranch master
-        PYTHONPATH=`pwd` pytest -s -v test
+        git config --global --add advice.detachedHead true
+        ${{ matrix.os == 'windows-latest' && 'set PYTHONPATH=%cd% &&' || 'PYTHONPATH=`pwd`' }} pytest -s -v test

test/test_commands.py

@@ -9,8 +9,10 @@
 from vcstool.clients.git import GitClient  # noqa: E402
 from vcstool.util import rmtree  # noqa: E402
 
+file_uri_scheme = 'file://' if sys.platform != 'win32' else 'file:///'
+
 REPOS_FILE = os.path.join(os.path.dirname(__file__), 'list.repos')
-REPOS_FILE_URL = 'file://' + REPOS_FILE
+REPOS_FILE_URL = file_uri_scheme + REPOS_FILE
 REPOS2_FILE = os.path.join(os.path.dirname(__file__), 'list2.repos')
 TEST_WORKSPACE = os.path.join(
     os.path.dirname(os.path.dirname(__file__)), 'test_workspace')
@@ -189,6 +191,11 @@ def test_pull_api(self):
 invocation.
 """
             output = output.replace(pull_warning, '')
+        # the output was retrieved through a different way here
+        output = adapt_command_output(output.encode()).decode()
+        if sys.platform == 'win32':
+            # it does not include carriage return characters on Windows
+            output = output.replace('\n', '\r\n')
         expected = get_expected_output('pull').decode()
         assert output == expected
 
@@ -216,6 +223,12 @@ def test_reimport(self):
         output = run_command(
             'import', ['--force', '--input', REPOS_FILE, '.'])
         expected = get_expected_output('reimport_force')
+        # on Windows, the "Already on 'master'" message is after the
+        # "Your branch is up to date with ..." message, so remove it
+        # from both output and expected strings
+        if sys.platform == 'win32':
+            output = output.replace(b"Already on 'master'\r\n", b'')
+            expected = expected.replace(b"Already on 'master'\r\n", b'')
         # newer git versions don't append three dots after the commit hash
         assert output == expected or output == expected.replace(b'... ', b' ')
 
@@ -352,6 +365,11 @@ def run_command(command, args=None, subfolder=None):
     output = subprocess.check_output(
         [sys.executable, script] + (args or []),
         stderr=subprocess.STDOUT, cwd=cwd, env=env)
+    return adapt_command_output(output, cwd)
+
+
+def adapt_command_output(output, cwd=None):
+    assert type(output) == bytes
     # replace message from older git versions
     output = output.replace(
         b'git checkout -b new_branch_name',
@@ -386,6 +404,23 @@ def run_command(command, args=None, subfolder=None):
     # replace GitHub SSH clone URL
     output = output.replace(
         b'[email protected]:', b'https://github.com/')
+    if sys.platform == 'win32':
+        if cwd:
+            # on Windows, git prints full path to repos
+            # in some messages, so make it relative
+            cwd_abs = os.path.abspath(cwd).replace('\\', '/')
+            output = output.replace(cwd_abs.encode(), b'.')
+        # replace path separators in specific paths;
+        # this is less likely to cause wrong test results
+        paths_to_replace = [
+            (b'.\\immutable', b'./immutable'),
+            (b'.\\vcstool', b'./vcstool'),
+            (b'.\\without_version', b'./without_version'),
+            (b'\\hash', b'/hash'),
+            (b'\\tag', b'/tag'),
+        ]
+        for before, after in paths_to_replace:
+            output = output.replace(before, after)
     return output