Fix MS-Windows specific failures in test suite. (#688)

Hi,

could you please review changes to fix the test suite to pass on
MS-Windows. It fixes #687.

It also enables the test suite which includes MS-Windows support to pass
in #686.

In addition to the document changes, the patch also
- Releases file/dir before removing (monkeypatch.chdir, os/close).
MS-Windows will complain about trying to remove a file that is still
being used otherwise.
- Add empty server to file: uri (i.e. file:///), since paths such as
c:\dev, will pickup c: as the server causing the tests to fail.
- Just looks for the file name in traceback texts (e.g.
*test_testrunner.lpy instead of /*/test_testrunner.lpy), otherwise it
will fail because the output string will be c:\*\test_testrunner.lpy on
windows.

The test suite currently fails to run because package dependencies have
moved since it was last run. Perhaps is best to merge this after #686.

Thanks

---------

Co-authored-by: ikappaki <[email protected]>
Co-authored-by: Chris Rink <[email protected]>

Author: ikappaki

.github/workflows/run-tests.yml

@@ -12,9 +12,10 @@ concurrency:
 
 jobs:
   run-tests:
-    runs-on: ubuntu-latest
+    runs-on: ${{matrix.os}}
     strategy:
       matrix:
+        os: [ubuntu-latest]
         version: ['3.8', '3.9', '3.10', '3.11']
         include:
           - version: '3.8'
@@ -25,6 +26,12 @@ jobs:
             tox-env: py310,py310-mypy,py310-lint,safety
           - version: '3.11'
             tox-env: py311,py311-mypy,py311-lint,format,safety
+          - os: windows-latest
+            version: '3.11'
+            tox-env: py311,safety
+          - os: macos-latest
+            version: '3.11'
+            tox-env: py311,safety
     steps:
       - uses: actions/checkout@v3
       - uses: actions/setup-python@v4
@@ -41,8 +48,13 @@ jobs:
             ~/.local/share/pypoetry
           key: ${{ runner.os }}-python-${{ matrix.version }}-poetry-${{ hashFiles('pyproject.toml', 'tox.ini') }}
       - name: Install Poetry
-        if: steps.cache-deps.outputs.cache-hit != 'true'
+        if: steps.cache-deps.outputs.cache-hit != 'true' && ! startsWith (matrix.os, 'windows')
         run: curl -sSL https://install.python-poetry.org | python3 -
+      - name: Install Poetry (Windows)
+        if: steps.cache-deps.outputs.cache-hit != 'true' && startsWith (matrix.os, 'windows')
+        shell: pwsh
+        run: |
+          (Invoke-WebRequest -Uri https://install.python-poetry.org -UseBasicParsing).Content | py -
       - name: Install Tox
         run: |
           pip install -U pip
@@ -57,6 +69,7 @@ jobs:
           mkdir coverage
           mv .coverage.* "coverage/.coverage.py${{ matrix.version }}"
       - name: Archive code coverage results
+        if: "startsWith (matrix.os, 'ubuntu')"
         uses: actions/upload-artifact@v3
         with:
           name: code-coverage
@@ -98,4 +111,4 @@ jobs:
       - name: Report Combined Coverage
         uses: coverallsapp/github-action@v2
         with:
-          file: coverage.xml
+          file: coverage.xml

src/basilisp/io.lpy

@@ -6,6 +6,7 @@
   interacting with the filesystem."
   (:import
    io
+   os.path
    pathlib
    shutil
    urllib.parse
@@ -31,7 +32,13 @@
   urllib.parse/ParseResult
   (as-path [f]
     (if (contains? #{"file" ""} (.-scheme f))
-      (-> (.-path f) (pathlib/Path))
+      (let [path (.-path f)]
+        (if (= sys/platform "win32")
+          ;; On MS-Windows, extracting the path from the URL
+          ;; incorrectly adds a leading `/', .e.g. /C:\xyz.
+          (pathlib/Path (subs path 1))
+
+          (pathlib/Path path)))
       (throw
        (ex-info "Cannot coerce non-File URL to pathlib.Path"
                 {:file f})))))
@@ -121,6 +128,18 @@
 
     Callers should generally prefer :lpy:fn:`output-stream` to this function."))
 
+(defn- convert-to-path-or-url [f-str]
+  "Convert ``f-str`` to a python Path or URL object based on to
+  whether it represents an absolute path or not, respectively.
+
+  This fn is intended to be used with the input and output
+  writers. Converting MS-Windows absolutely paths (such as c:\\xyz and
+  \\\\share\\xyz) directly to URLs are likely to confuse the urllib
+  parser. As such, they are converted to Path objects instead."
+  (if (os.path/isabs f-str)
+    (pathlib/Path f-str)
+    (urllib.parse/urlparse f-str)))
+
 (extend-protocol IOFactory
   io/TextIOBase
   (make-reader [f opts]
@@ -191,22 +210,22 @@
   python/str
   (make-reader [f opts]
     (try
-      (make-reader (urllib.parse/urlparse f) opts)
+      (make-reader (convert-to-path-or-url f) opts)
       (catch python/ValueError _
         (make-reader (pathlib/Path f) opts))))
   (make-writer [f opts]
     (try
-      (make-writer (urllib.parse/urlparse f) opts)
+      (make-writer (convert-to-path-or-url f) opts)
       (catch python/ValueError _
         (make-writer (pathlib/Path f) opts))))
   (make-input-stream [f opts]
     (try
-      (make-input-stream (urllib.parse/urlparse f) opts)
+      (make-input-stream (convert-to-path-or-url f) opts)
       (catch python/ValueError _
         (make-input-stream (pathlib/Path f) opts))))
   (make-output-stream [f opts]
     (try
-      (make-output-stream (urllib.parse/urlparse f) opts)
+      (make-output-stream (convert-to-path-or-url f) opts)
       (catch python/ValueError _
         (make-output-stream (pathlib/Path f) opts))))
 

tests/basilisp/compiler_test.py

@@ -5405,6 +5405,7 @@ def test_aliased_namespace_not_hidden_by_python_module(
         self, lcompile: CompileFn, monkeypatch: MonkeyPatch
     ):
         with TemporaryDirectory() as tmpdir:
+            cwd = os.getcwd()
             monkeypatch.chdir(tmpdir)
             monkeypatch.syspath_prepend(tmpdir)
             monkeypatch.setattr(
@@ -5432,6 +5433,7 @@ def test_aliased_namespace_not_hidden_by_python_module(
                 """
                 )
             finally:
+                monkeypatch.chdir(cwd)
                 os.unlink(module_file_path)
 
     def test_aliased_var_does_not_resolve(

tests/basilisp/importer_test.py

@@ -99,10 +99,12 @@ def module_cache(self):
     @pytest.fixture
     def module_dir(self, monkeypatch: MonkeyPatch, module_cache):
         with TemporaryDirectory() as tmpdir:
+            cwd = os.getcwd()
             monkeypatch.chdir(tmpdir)
             monkeypatch.syspath_prepend(tmpdir)
             monkeypatch.setattr("sys.modules", module_cache)
             yield tmpdir
+            monkeypatch.chdir(cwd)
 
     @pytest.fixture
     def make_new_module(self, module_dir):

tests/basilisp/test_core_macros.lpy

@@ -443,13 +443,14 @@
   (is (= #py ["A" "B" "C"] (.. "a,b,c" upper (split ",")))))
 
 (deftest with-test
-  (let [[_ filename] (tempfile/mkstemp)]
+  (let [[fh filename] (tempfile/mkstemp)]
     (try
       (with-open [file (python/open filename "w")]
         (.write file "Testing text"))
       (with-open [file (python/open filename)]
         (is (= "Testing text" (.read file))))
       (finally
+        (os/close fh)
         (os/unlink filename)))))
 
 (deftest if-let-test

tests/basilisp/test_io.lpy

@@ -7,6 +7,7 @@
    pathlib
    tempfile
    threading
+   time
    urllib.parse
    urllib.request)
   (:require
@@ -82,12 +83,12 @@
       (try
         (spit filename "hi there")
         (is (= "hi there" (slurp filename)))
-        (is (= "hi there" (slurp (str "file://" filename))))
+        (is (= "hi there" (slurp (str "file:///" filename))))
         (finally
           (os/close fd)
           (os/unlink filename)))))
 
-  (testing "local files"
+   (testing "local files"
     (let [path (bio/path *tempdir* "reader-test-local-files.txt")]
       (spit path "some content")
 
@@ -111,6 +112,9 @@
                                        *http-port*
                                        "/reader-http-req.txt"))))
         (finally
+          ;; Give a chance to the server to release the
+          ;; file before removing it.
+          (time/sleep 1)
           (os/unlink filename))))))
 
 (deftest writer-test
@@ -196,7 +200,7 @@
       (try
         (spit-bytes filename "hi there")
         (is (= (bytes "hi there") (slurp-bytes filename)))
-        (is (= (bytes "hi there") (slurp-bytes (str "file://" filename))))
+        (is (= (bytes "hi there") (slurp-bytes (str "file:///" filename))))
         (finally
           (os/close fd)
           (os/unlink filename)))))
@@ -226,6 +230,9 @@
                                        *http-port*
                                        "/input-streams-http-req.txt"))))
         (finally
+          ;; Give a chance to the server to release the
+          ;; file before removing it.
+          (time/sleep 1)
           (os/unlink filename))))))
 
 (deftest output-stream-test

tests/basilisp/testrunner_test.py

@@ -94,7 +94,7 @@ def test_error_repr(self, run_result: RunResult):
                 "ERROR in (assertion-test) (test_testrunner.lpy:12)",
                 "",
                 "Traceback (most recent call last):",
-                '  File "/*/test_testrunner.lpy", line 12, in assertion_test',
+                '  File "*test_testrunner.lpy", line 12, in assertion_test',
                 '    (is (throw (ex-info "Uncaught exception" {}))))',
                 "basilisp.lang.exception.ExceptionInfo: Uncaught exception {}",
             ]
@@ -118,7 +118,7 @@ def test_error_repr(self, run_result: RunResult):
             [
                 "ERROR in (error-test) (test_testrunner.lpy)",
                 "Traceback (most recent call last):",
-                '  File "/*/test_testrunner.lpy", line 25, in error_test',
+                '  File "*test_testrunner.lpy", line 25, in error_test',
                 "    (throw",
                 "basilisp.lang.exception.ExceptionInfo: This test will count as an error. {}",
             ]