Chore: Get fast-test working on Windows (#4467)

Author: erindru

.circleci/continue_config.yml

@@ -99,35 +99,35 @@ jobs:
       - store_test_results:
           path: test-results
 
-  style_and_cicd_tests_windows:
+  cicd_tests_windows:
     executor:
       name: windows/default
       size: large
-    environment:
-      PYTEST_XDIST_AUTO_NUM_WORKERS: 4
     steps:
       - halt_unless_core
+      - run:
+          name: Enable symlinks in git config
+          command: git config --global core.symlinks true
       - checkout
       - run:
-          name: Install Python 3.9
+          name: Install System Dependencies
           command: |
-            choco install python --version=3.9 -y
+            choco install make which -y
             refreshenv
-      - run: 
-          name: Install make
-          command: choco install make -y
       - run:
           name: Install SQLMesh dev dependencies
-          command: make install-dev
-      - run:
-          name: Fix Git URL override
-          command: git config --global --unset url."ssh://git@github.com".insteadOf
-      # - run:
-      #     name: Run linters and code style checks
-      #     command: make py-style
+          command: |
+            python -m venv venv
+            . ./venv/Scripts/activate
+            python.exe -m pip install --upgrade pip
+            make install-dev
       - run:
-          name: Run cicd tests
-          command: pytest -n auto -m "fast" --junitxml=test-results/junit-cicd.xml tests/lsp
+          name: Run fast unit tests
+          command: |
+            . ./venv/Scripts/activate
+            which python
+            python --version
+            make fast-test
       - store_test_results:
           path: test-results
 
@@ -304,7 +304,7 @@ workflows:
                 - "3.10"
                 - "3.11"
                 - "3.12"
-      - style_and_cicd_tests_windows
+      - cicd_tests_windows
       - engine_tests_docker:
           name: engine_<< matrix.engine >>
           matrix:

Makefile

@@ -64,7 +64,7 @@ engine-up: engine-clickhouse-up engine-mssql-up engine-mysql-up engine-postgres-
 engine-down: engine-clickhouse-down engine-mssql-down engine-mysql-down engine-postgres-down engine-spark-down engine-trino-down
 
 fast-test:
-	pytest -n auto -m "fast and not cicdonly" && pytest -m "isolated"
+	pytest -n auto -m "fast and not cicdonly" --junitxml=test-results/junit-fast-test.xml && pytest -m "isolated"
 
 slow-test:
 	pytest -n auto -m "(fast or slow) and not cicdonly" && pytest -m "isolated"

examples/sushi/linter/__init__.py

@@ -0,0 +1 @@
+# this makes "linter" a package so "linter.user" is a valid module for importlib.import_module() to load

sqlmesh/core/engine_adapter/athena.py

@@ -8,7 +8,7 @@
 from sqlmesh.core.engine_adapter.mixins import PandasNativeFetchDFSupportMixin, RowDiffMixin
 from sqlmesh.core.engine_adapter.trino import TrinoEngineAdapter
 from sqlmesh.core.node import IntervalUnit
-import os
+import posixpath
 from sqlmesh.utils.errors import SQLMeshError
 from sqlmesh.core.engine_adapter.shared import (
     CatalogSupport,
@@ -403,11 +403,13 @@ def _table_location(
 
         elif self.s3_warehouse_location:
             # If the user has set `s3_warehouse_location` in the connection config, the base URI is <s3_warehouse_location>/<catalog>/<schema>/
-            base_uri = os.path.join(self.s3_warehouse_location, table.catalog or "", table.db or "")
+            base_uri = posixpath.join(
+                self.s3_warehouse_location, table.catalog or "", table.db or ""
+            )
         else:
             return None
 
-        full_uri = validate_s3_uri(os.path.join(base_uri, table.text("this") or ""), base=True)
+        full_uri = validate_s3_uri(posixpath.join(base_uri, table.text("this") or ""), base=True)
         return exp.LocationProperty(this=exp.Literal.string(full_uri))
 
     def _find_matching_columns(

sqlmesh/core/model/cache.py

@@ -15,6 +15,7 @@
 from sqlmesh.core.model.definition import ExternalModel, Model, SqlModel, _Model
 from sqlmesh.utils.cache import FileCache
 from sqlmesh.utils.hashing import crc32
+from sqlmesh.utils.windows import IS_WINDOWS
 
 from dataclasses import dataclass
 
@@ -149,8 +150,10 @@ def _entry_name(model: SqlModel) -> str:
 
 
 def optimized_query_cache_pool(optimized_query_cache: OptimizedQueryCache) -> ProcessPoolExecutor:
+    # fork doesnt work on Windows. ref: https://docs.python.org/3/library/multiprocessing.html#multiprocessing-start-methods
+    context_type = "spawn" if IS_WINDOWS else "fork"
     return ProcessPoolExecutor(
-        mp_context=mp.get_context("fork"),
+        mp_context=mp.get_context(context_type),
         initializer=_init_optimized_query_cache,
         initargs=(optimized_query_cache,),
         max_workers=c.MAX_FORK_WORKERS,

sqlmesh/utils/cache.py

@@ -12,6 +12,7 @@
 from sqlmesh.utils import sanitize_name
 from sqlmesh.utils.date import to_datetime
 from sqlmesh.utils.errors import SQLMeshError
+from sqlmesh.utils.windows import IS_WINDOWS, fix_windows_path
 
 logger = logging.getLogger(__name__)
 
@@ -132,4 +133,8 @@ def clear(self) -> None:
 
     def _cache_entry_path(self, name: str, entry_id: str = "") -> Path:
         entry_file_name = "__".join(p for p in (self._cache_version, name, entry_id) if p)
-        return self._path / sanitize_name(entry_file_name)
+        full_path = self._path / sanitize_name(entry_file_name)
+        if IS_WINDOWS:
+            # handle paths longer than 260 chars
+            full_path = fix_windows_path(full_path)
+        return full_path

sqlmesh/utils/windows.py

@@ -0,0 +1,14 @@
+import platform
+from pathlib import Path
+
+IS_WINDOWS = platform.system() == "Windows"
+
+
+def fix_windows_path(path: Path) -> Path:
+    """
+    Windows paths are limited to 260 characters: https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation
+    Users can change this by updating a registry entry but we cant rely on that.
+    We can quite commonly generate a cache file path that exceeds 260 characters which causes a FileNotFound error.
+    If we prefix the path with "\\?\" then we can have paths up to 32,767 characters
+    """
+    return Path("\\\\?\\" + str(path.absolute()))

tests/conftest.py

@@ -9,6 +9,8 @@
 from tempfile import TemporaryDirectory
 from unittest import mock
 from unittest.mock import PropertyMock
+import os
+import shutil
 
 import duckdb
 import pandas as pd
@@ -39,6 +41,7 @@
 )
 from sqlmesh.utils import random_id
 from sqlmesh.utils.date import TimeLike, to_date
+from sqlmesh.utils.windows import IS_WINDOWS, fix_windows_path
 from sqlmesh.core.engine_adapter.shared import CatalogSupport
 
 T = t.TypeVar("T", bound=EngineAdapter)
@@ -480,10 +483,27 @@ def _make_function(
         paths: t.Union[t.Union[str, Path], t.Collection[t.Union[str, Path]]],
     ) -> t.List[Path]:
         paths = ensure_list(paths)
+        all_paths = [Path(p) for p in paths]
         temp_dirs = []
-        for path in paths:
+        for path in all_paths:
             temp_dir = Path(tmp_path) / uuid.uuid4().hex
-            copytree(path, temp_dir, symlinks=True, ignore=ignore)
+
+            if IS_WINDOWS:
+                # shutil.copytree just doesnt work properly with the symlinks on Windows, regardless of the `symlinks` setting
+                src = str(path.absolute())
+                dst = str(temp_dir.absolute())
+                os.system(f"robocopy {src} {dst} /E /COPYALL")
+
+                # after copying, delete the files that would have been ignored
+                for root, dirs, _ in os.walk(temp_dir):
+                    for dir in dirs:
+                        full_dir = fix_windows_path(Path(root) / dir)
+                        for ignored in ignore(full_dir, [full_dir]):
+                            shutil.rmtree(ignored)
+
+            else:
+                copytree(path, temp_dir, symlinks=True, ignore=ignore)
+
             temp_dirs.append(temp_dir)
         return temp_dirs
 

tests/core/test_format.py

@@ -61,10 +61,10 @@ def test_format_files(tmp_path: pathlib.Path, mocker: MockerFixture):
     assert all(
         c in context.console.log_status_update.mock_calls  # type: ignore
         for c in [
-            call(f"{tmp_path}/models/model_3.sql needs reformatting."),
-            call(f"{tmp_path}/models/model_2.sql needs reformatting."),
-            call(f"{tmp_path}/models/model_1.sql needs reformatting."),
-            call(f"{tmp_path}/audits/audit_1.sql needs reformatting."),
+            call(f"{tmp_path / 'models/model_3.sql'} needs reformatting."),
+            call(f"{tmp_path / 'models/model_2.sql'} needs reformatting."),
+            call(f"{tmp_path / 'models/model_1.sql'} needs reformatting."),
+            call(f"{tmp_path / 'audits/audit_1.sql'} needs reformatting."),
             call("\n4 file(s) need reformatting."),
         ]
     )

tests/core/test_model.py

@@ -1238,7 +1238,7 @@ def test_seed_marker_substitution():
     )
 
     assert isinstance(model.kind, SeedKind)
-    assert model.kind.path == "examples/sushi/seeds/waiter_names.csv"
+    assert model.kind.path == str(Path("examples/sushi/seeds/waiter_names.csv"))
     assert model.seed is not None
     assert len(model.seed.content) > 0