Fix windows tests (#2025)

* Fix benchmarks on Windows

* Fix tests for Windows

+ Improve failure messages

* Add CI job for Windows

* Disable unused warning for ParseFormatError

* Add `pprof` feature to utils

.github/workflows/continuous-integration.yml

@@ -83,3 +83,18 @@ jobs:
 
     - name: Typecheck benchmarks
       run: find core/benches -type f -name "*.ncl" -print0 | xargs -0 -I file nix run . -- typecheck file
+
+  build-and-test-windows:
+    name: "build-and-test (windows-latest, stable)"
+    runs-on: windows-latest
+    continue-on-error: true
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: actions-rust-lang/setup-rust-toolchain@v1
+
+      - name: Build
+        run: cargo build --all-targets --package nickel-lang-*
+
+      - name: Test
+        run: cargo test --package nickel-lang-*

cli/tests/snapshot/main.rs

@@ -16,8 +16,9 @@ macro_rules! assert_snapshot_filtered {
             // Since error output includes fully-qualified paths to the source file
             // we need to replace those with something static to avoid snapshots
             // differing across machines.
-            (r"(?:/.+/tests/snapshot/inputs)", "[INPUTS_PATH]"),
-            (r"(?:/.+/tests/snapshot/imports)", "[IMPORTS_PATH]")
+            // Replace filepath backslashes with forward slashes for Windows.
+            (r"(?:(?:.:)?(?:/|\\).+(?:/|\\)tests(?:/|\\)snapshot(?:/|\\)inputs(?:/|\\))([0-9A-Za-z_-]+)(?:/|\\)(.+)", "[INPUTS_PATH]/${1}/${2}"),
+            (r"(?:(?:.:)?(?:/|\\).+(?:/|\\)tests(?:/|\\)snapshot(?:/|\\)imports(?:/|\\))(.+)", "[IMPORTS_PATH]/${1}")
         ]},
         {
             insta::assert_snapshot!($name, $snapshot);

core/Cargo.toml

@@ -86,8 +86,7 @@ strsim = "0.10.0"
 pretty_assertions.workspace = true
 assert_matches.workspace = true
 criterion.workspace = true
-pprof = { workspace = true, features = ["criterion", "flamegraph"] }
-nickel-lang-utils.workspace = true
+nickel-lang-utils = { workspace = true, features = ["pprof"] }
 similar.workspace = true
 test-generator.workspace = true
 

core/benches/arrays.rs

@@ -1,12 +1,11 @@
 use std::rc::Rc;
 
-use criterion::{criterion_main, Criterion};
+use criterion::criterion_main;
 use nickel_lang_core::term::{
     array::{Array, ArrayAttrs},
     Number, RichTerm, Term,
 };
-use nickel_lang_utils::{bench::EvalMode, ncl_bench_group};
-use pprof::criterion::{Output, PProfProfiler};
+use nickel_lang_utils::{bench::criterion_config, bench::EvalMode, ncl_bench_group};
 use pretty::{BoxAllocator, DocBuilder, Pretty};
 
 /// Generates a pseaudo-random Nickel array as a string.
@@ -35,7 +34,7 @@ fn ncl_random_array(len: usize) -> String {
 
 ncl_bench_group! {
 name = benches;
-config = Criterion::default().with_profiler(PProfProfiler::new(100, Output::Flamegraph(None)));
+config = criterion_config();
 {
         name = "foldr strings 50",
         path = "arrays/fold",

core/benches/functions.rs

@@ -1,10 +1,9 @@
-use criterion::{criterion_main, Criterion};
-use nickel_lang_utils::ncl_bench_group;
-use pprof::criterion::{Output, PProfProfiler};
+use criterion::criterion_main;
+use nickel_lang_utils::{bench::criterion_config, ncl_bench_group};
 
 ncl_bench_group! {
     name = benches;
-    config = Criterion::default().with_profiler(PProfProfiler::new(100, Output::Flamegraph(None)));
+    config = criterion_config();
     {
         name = "church 3",
         path = "functions/church",

core/benches/mantis.rs

@@ -1,10 +1,9 @@
-use criterion::{criterion_main, Criterion};
-use nickel_lang_utils::{bench::EvalMode, ncl_bench_group};
-use pprof::criterion::{Output, PProfProfiler};
+use criterion::criterion_main;
+use nickel_lang_utils::{bench::criterion_config, bench::EvalMode, ncl_bench_group};
 
 ncl_bench_group! {
     name = benches;
-    config = Criterion::default().with_profiler(PProfProfiler::new(100, Output::Flamegraph(None)));
+    config = criterion_config();
     {
         name = "mantis",
         path = "mantis/run",

core/benches/numeric.rs

@@ -1,10 +1,9 @@
-use criterion::{criterion_main, Criterion};
-use nickel_lang_utils::ncl_bench_group;
-use pprof::criterion::{Output, PProfProfiler};
+use criterion::criterion_main;
+use nickel_lang_utils::{bench::criterion_config, ncl_bench_group};
 
 ncl_bench_group! {
     name = benches;
-    config = Criterion::default().with_profiler(PProfProfiler::new(100, Output::Flamegraph(None)));
+    config = criterion_config();
     {
         name = "fibonacci 10",
         path = "numeric/fibonacci",

core/benches/records.rs

@@ -1,10 +1,9 @@
-use criterion::{criterion_main, Criterion};
-use nickel_lang_utils::{bench::EvalMode, ncl_bench_group};
-use pprof::criterion::{Output, PProfProfiler};
+use criterion::criterion_main;
+use nickel_lang_utils::{bench::criterion_config, bench::EvalMode, ncl_bench_group};
 
 ncl_bench_group! {
     name = benches;
-    config = Criterion::default().with_profiler(PProfProfiler::new(100, Output::Flamegraph(None)));
+    config = criterion_config();
     {
             name = "countLetters",
             path = "records/countLetters",

core/benches/serialization.rs

@@ -1,10 +1,9 @@
-use criterion::{criterion_main, Criterion};
-use nickel_lang_utils::ncl_bench_group;
-use pprof::criterion::{Output, PProfProfiler};
+use criterion::criterion_main;
+use nickel_lang_utils::{bench::criterion_config, ncl_bench_group};
 
 ncl_bench_group! {
     name = benches;
-    config = Criterion::default().with_profiler(PProfProfiler::new(100, Output::Flamegraph(None)));
+    config = criterion_config();
     {
         name = "round_trip",
         path = "serialization/main",

core/benches/stdlib.rs

@@ -1,7 +1,7 @@
 use criterion::{criterion_group, criterion_main, Criterion};
-use pprof::criterion::{Output, PProfProfiler};
 
 use nickel_lang_core::cache::{Cache, ErrorTolerance};
+use nickel_lang_utils::bench::criterion_config;
 
 pub fn typecheck_stdlib(c: &mut Criterion) {
     let mut cache = Cache::new(ErrorTolerance::Strict);
@@ -18,7 +18,7 @@ pub fn typecheck_stdlib(c: &mut Criterion) {
 
 criterion_group!(
 name = benches;
-config = Criterion::default().with_profiler(PProfProfiler::new(100, Output::Flamegraph(None)));
+config = criterion_config();
 targets = typecheck_stdlib
 );
 criterion_main!(benches);

core/benches/typecheck-nixpkgs-lib.rs

@@ -1,10 +1,9 @@
-use criterion::{criterion_main, Criterion};
-use nickel_lang_utils::{bench::EvalMode, ncl_bench_group};
-use pprof::criterion::{Output, PProfProfiler};
+use criterion::criterion_main;
+use nickel_lang_utils::{bench::criterion_config, bench::EvalMode, ncl_bench_group};
 
 ncl_bench_group! {
     name = benches;
-    config = Criterion::default().with_profiler(PProfProfiler::new(100, Output::Flamegraph(None)));
+    config = criterion_config();
     {
         name = "nixpkgs lists",
         path = "nixpkgs/lists",

core/src/serialize.rs

@@ -47,6 +47,8 @@ impl fmt::Display for ExportFormat {
     }
 }
 
+// TODO: This type is publicly exposed, but never constructed.
+#[allow(dead_code)]
 #[derive(Clone, Eq, PartialEq, Debug)]
 pub struct ParseFormatError(String);
 

lsp/lsp-harness/src/lib.rs

@@ -5,7 +5,6 @@ use std::collections::{hash_map::Entry, HashMap};
 
 use assert_cmd::prelude::CommandCargoExt;
 pub use jsonrpc::Server;
-use log::error;
 use lsp_types::{
     notification::{Notification, PublishDiagnostics},
     request::{
@@ -57,23 +56,94 @@ pub struct Requests {
     diagnostic: Option<Vec<Url>>,
 }
 
+/// Produce an absolute filepath `Url` that is safe to use in tests.
+///
+/// The `C:\` prefix on Windows is needed both to avoid `Url::from_file_path` failing due to a
+/// missing drive letter and to avoid invalid filepath errors.
+pub fn file_url_from_path(path: &str) -> Result<Url, String> {
+    assert!(path.starts_with('/'));
+
+    let path = if cfg!(unix) {
+        path.to_owned()
+    } else {
+        format!("C:\\{}", &path[1..])
+    };
+
+    Url::from_file_path(&path).map_err(|()| format!("Unable to convert filepath {path:?} into Url"))
+}
+
+#[cfg(windows)]
+fn modify_requests_uris(mut reqs: Requests) -> Requests {
+    match reqs.request.iter_mut().next() {
+        None => {}
+        Some(rs) => {
+            for req in rs.iter_mut() {
+                modify_request_uri(req);
+            }
+        }
+    }
+    match reqs.diagnostic.iter_mut().next() {
+        None => {}
+        Some(urls) => {
+            for url in urls.iter_mut() {
+                *url = file_url_from_path(url.path()).unwrap();
+            }
+        }
+    };
+    reqs
+}
+
+#[cfg(windows)]
+fn modify_request_uri(req: &mut Request) {
+    fn file_url(url: &Url) -> Url {
+        file_url_from_path(url.path()).unwrap()
+    }
+
+    match req {
+        Request::GotoDefinition(params) => {
+            params.text_document_position_params.text_document.uri =
+                file_url(&params.text_document_position_params.text_document.uri);
+        }
+        Request::References(params) => {
+            params.text_document_position.text_document.uri =
+                file_url(&params.text_document_position.text_document.uri);
+        }
+        Request::Completion(params) => {
+            params.text_document_position.text_document.uri =
+                file_url(&params.text_document_position.text_document.uri);
+        }
+        Request::Formatting(params) => {
+            params.text_document.uri = file_url(&params.text_document.uri);
+        }
+        Request::Hover(params) => {
+            params.text_document_position_params.text_document.uri =
+                file_url(&params.text_document_position_params.text_document.uri);
+        }
+        Request::Rename(params) => {
+            params.text_document_position.text_document.uri =
+                file_url(&params.text_document_position.text_document.uri);
+        }
+        Request::Symbols(params) => {
+            params.text_document.uri = file_url(&params.text_document.uri);
+        }
+    }
+}
+
 impl TestFixture {
-    pub fn parse(s: &str) -> Option<Self> {
+    pub fn parse(s: &str) -> Result<Self, String> {
         let mut header_lines = Vec::new();
         let mut content = String::new();
         let mut files = Vec::new();
         let mut push_file = |header: &[&str], content: &mut String| {
-            if header.len() > 1 {
-                error!("files can only have 1 header line");
-                return None;
-            }
-
-            let uri = Url::from_file_path(header.first()?).ok()?;
+            let uri = match header {
+                &[path] => file_url_from_path(path),
+                _ => Err(format!("Files can only have 1 header line: {header:?}")),
+            }?;
             files.push(TestFile {
                 uri,
                 contents: std::mem::take(content),
             });
-            Some(())
+            Ok::<(), String>(())
         };
 
         for line in s.lines() {
@@ -94,7 +164,7 @@ impl TestFixture {
             // The text fixture ended with a nickel file; there are no lsp
             // requests specified.
             push_file(&header_lines, &mut content)?;
-            Some(TestFixture {
+            Ok(TestFixture {
                 files,
                 reqs: Vec::new(),
                 expected_diags: Vec::new(),
@@ -105,7 +175,10 @@ impl TestFixture {
             // we expect to receive.
             let remaining = header_lines.join("\n");
             let reqs: Requests = toml::from_str(&remaining).unwrap();
-            Some(TestFixture {
+            #[cfg(windows)]
+            let reqs = modify_requests_uris(reqs);
+
+            Ok(TestFixture {
                 files,
                 reqs: reqs.request.unwrap_or_default(),
                 expected_diags: reqs.diagnostic.unwrap_or_default(),

lsp/nls/Cargo.toml

@@ -58,7 +58,7 @@ assert_cmd.workspace = true
 assert_matches.workspace = true
 criterion.workspace = true
 glob = "0.3.1"
-insta.workspace = true
+insta = { workspace = true, features = ["filters"] }
 lsp-harness.workspace = true
 nickel-lang-utils.workspace = true
 pretty_assertions.workspace = true

lsp/nls/benches/main.rs

@@ -53,7 +53,8 @@ fn test_requests(c: &mut Criterion) {
 fn benchmark_one_test(c: &mut Criterion, path: &str) {
     let full_path = project_root().join(path);
     let contents = std::fs::read_to_string(&full_path).unwrap();
-    let fixture = TestFixture::parse(&contents).unwrap();
+    let fixture =
+        TestFixture::parse(&contents).unwrap_or_else(|s| panic!("Failed parsing {path:?}: {s}"));
     let mut harness = TestHarness::new();
 
     harness.prepare_files(&fixture);