Add testing on Windows, macOS, and R 4.2 (#545)

* Explore testing on windows

* Don't need `r-base-dev`, right?

* Fix typo

* I think `cargo build` before `cargo test` is redundant?

* Write in binary mode to ensure `eol` is respected on Windows

* Ensure file is closed before getting file size

* Can it even roundtrip correctly?

* Did it get into R right?

* See if `&str` to `RObject` loses encoding

* What is the encoding?

* What about the windows version?

* make sure build.rs is actually being called

* panic after to get stderr output

* Remove panic

* does this happen to work?

* what does an integration test report?

* Use `compile_for_everything()` to embed the manifest in the test binary

And also do this for the harp test binary, which starts R and tests UTF-8 related capabilities.

* Try removing some extra installs

* Create unified `test.yml`

* Make the subworkflows callable

* Add script to auto generate the manifest files

* Post rebase fixup

* Only test on nightly on 1 OS, also test on R 4.2

* Less repetition

* Add in macOS testing

* An `origin` is required on R <= 4.2 for `as.POSIXct.numeric()`

* Looks better without this too

* Allow SSH access on workflow dispatch

* Try and get some information on where `recv()` is hanging

* Send timeout too

* Remove timeout code for the moment

* Install R packages for tests

* Add a note on the application manifest

* Reference manifest files

* It's `r_task()` everywhere now

.github/workflows/test-linux.yml

@@ -0,0 +1,60 @@
+name: "Test Ark - Linux"
+
+on:
+  workflow_call:
+  workflow_dispatch:
+    inputs:
+      ssh:
+        description: 'Set up an SSH session before running `cargo test`?'
+        type: boolean
+        required: true
+        default: false
+
+jobs:
+  linux:
+    runs-on: ubuntu-latest
+    name: "Rust: ${{ matrix.config.rust }}, R: ${{ matrix.config.r }}"
+    strategy:
+      fail-fast: false
+      matrix:
+        config:
+          - { rust: 'stable',  r: 'release' }
+          # Oldest supported R version
+          - { rust: 'stable',  r: '4.2' }
+          # Nightly rust
+          - { rust: 'nightly', r: 'release' }
+    timeout-minutes: 30
+    env:
+      GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: dtolnay/rust-toolchain@nightly
+        if: matrix.config.rust == 'nightly'
+
+      - name: Install R
+        uses: r-lib/actions/setup-r@v2
+        with:
+          r-version: ${{ matrix.config.r }}
+          use-public-rspm: true
+
+      - name: Install R Packages Required For Tests
+        uses: r-lib/actions/setup-r-dependencies@v2
+        with:
+          packages:
+            data.table
+            tibble
+
+      - name: Setup Build Environment
+        run: |
+          sudo apt-get update
+
+      - name: Setup SSH access
+        uses: mxschmitt/action-tmate@v3
+        if: ${{ inputs.ssh }}
+        timeout-minutes: 30
+
+      - name: Run Unit Tests
+        id: test
+        run: |
+            cargo test --verbose

.github/workflows/test-macos.yml

@@ -0,0 +1,38 @@
+name: "Test Ark - macOS"
+
+on:
+  workflow_call:
+  workflow_dispatch:
+
+jobs:
+  macos:
+    runs-on: macos-latest
+    name: "Rust: ${{ matrix.config.rust }}, R: ${{ matrix.config.r }}"
+    strategy:
+      fail-fast: false
+      matrix:
+        config:
+          - { rust: 'stable', r: 'release' }
+    timeout-minutes: 30
+    env:
+      GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Install R
+        uses: r-lib/actions/setup-r@v2
+        with:
+          r-version: ${{ matrix.config.r }}
+          use-public-rspm: true
+
+      - name: Install R Packages Required For Tests
+        uses: r-lib/actions/setup-r-dependencies@v2
+        with:
+          packages:
+            data.table
+            tibble
+
+      - name: Run Unit Tests
+        id: test
+        run: |
+            cargo test --verbose

.github/workflows/test-windows.yml

@@ -0,0 +1,38 @@
+name: "Test Ark - Windows"
+
+on:
+  workflow_call:
+  workflow_dispatch:
+
+jobs:
+  windows:
+    runs-on: windows-latest
+    name: "Rust: ${{ matrix.config.rust }}, R: ${{ matrix.config.r }}"
+    strategy:
+      fail-fast: false
+      matrix:
+        config:
+          - { rust: 'stable', r: 'release' }
+    timeout-minutes: 30
+    env:
+      GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Install R
+        uses: r-lib/actions/setup-r@v2
+        with:
+          r-version: ${{ matrix.config.r }}
+          use-public-rspm: true
+
+      - name: Install R Packages Required For Tests
+        uses: r-lib/actions/setup-r-dependencies@v2
+        with:
+          packages:
+            data.table
+            tibble
+
+      - name: Run Unit Tests
+        id: test
+        run: |
+            cargo test --verbose

.github/workflows/test.yml

@@ -0,0 +1,23 @@
+name: "Test Ark"
+on:
+  push:
+    branches:
+      - main
+  pull_request:
+  workflow_dispatch:
+
+jobs:
+  test_macos:
+    name: Test macOS
+    uses: ./.github/workflows/test-macos.yml
+    secrets: inherit
+
+  test_windows:
+    name: Test Windows
+    uses: ./.github/workflows/test-windows.yml
+    secrets: inherit
+
+  test_linux:
+    name: Test Linux
+    uses: ./.github/workflows/test-linux.yml
+    secrets: inherit

crates/ark/Cargo.toml

@@ -68,7 +68,7 @@ stdext = { path = "../stdext", features = ["testing"]}
 
 [build-dependencies]
 chrono = "0.4.23"
-embed-resource = "2.4.0"
+embed-resource = "2.5.0"
 
 [package.metadata.generate-rpm]
 assets = [{ source = "target/release/ark", dest = "/usr/bin/ark", mode = "755" }]

crates/ark/build.rs

@@ -36,9 +36,27 @@ fn main() {
     println!("cargo:rustc-env=BUILD_DATE={}", build_date);
 
     // Embed an Application Manifest file on Windows.
+    // Turns on UTF-8 support and declares our Windows version compatibility.
     // Documented to do nothing on non-Windows.
+    // See <crates/ark/resources/manifest/ark.exe.manifest>.
+    //
+    // We also do this for harp to support its unit tests.
+    //
+    // We can't just use `compile()`, as that uses `cargo:rustc-link-arg-bins`,
+    // which targets the main `ark.exe` (good) but not the test binaries (bad).
+    // We need the application manifest to get embedded into the ark/harp test
+    // binaries too, so that the instance of R started by our tests also has
+    // UTF-8 support.
+    //
+    // We can't use `compile_for_tests()` because `cargo:rustc-link-arg-tests`
+    // only targets integration tests right now, not unit tests.
+    // https://github.com/rust-lang/cargo/issues/10937
+    //
+    // Instead we use `compile_for_everything()` which uses the kitchen sink
+    // instruction of `cargo:rustc-link-arg`, and that seems to work.
+    // https://github.com/nabijaczleweli/rust-embed-resource/issues/69
     let resource = Path::new("resources")
         .join("manifest")
         .join("ark-manifest.rc");
-    embed_resource::compile(resource, embed_resource::NONE);
+    embed_resource::compile_for_everything(resource, embed_resource::NONE);
 }

crates/ark/resources/manifest/ark-manifest.rc

@@ -1,3 +1,5 @@
+// Autogenerated, do not modify! See `template.rc`.
+
 // This is a C file that embed-resource compiles for us.
 // It helps embed our Windows Application Manifest file.
 // https://learn.microsoft.com/en-us/windows/win32/sbscs/application-manifests
@@ -7,5 +9,5 @@
 // https://github.com/nabijaczleweli/rust-embed-resource/issues/11#issuecomment-779295722
 #include <winuser.h>
 
-// The `1` is a `resource_id` that specifies ark as an executable file
+// The `1` is a `resource_id` that specifies us as an executable file
 1 RT_MANIFEST "ark.exe.manifest"

crates/ark/resources/manifest/ark.exe.manifest

@@ -3,6 +3,7 @@
 
    <assemblyIdentity version="1.0.0.0" name="ark" type="win32"/>
 
+   <!-- Autogenerated, do not modify! See `template.manifest`. -->
    <description>ark</description>
 
    <!-- Support UTF-8 code page with R 4.2.0 and newer. -->

crates/ark/src/modules/positron/r_data_explorer.R

@@ -342,13 +342,38 @@ export_selection <- function(x, format = c("csv", "tsv", "html"), include_header
 }
 
 write_delim <- function(x, delim, include_header) {
-    tmp <- tempfile()
-    defer(unlink(tmp))
+    path <- tempfile()
+    defer(unlink(path))
 
-    utils::write.table(x, tmp, sep = delim, row.names = FALSE, col.names = include_header, quote = FALSE, na = "")
-    # We use size - 1 because we don't want to read the last newline character
-    # that creates problems when pasting the content in spreadsheets
-    readChar(tmp, file.info(tmp)$size - 1L)
+    write_delim_impl(x, path, delim, include_header)
+
+    # We use `size - 1` because we don't want to read the last newline character
+    # as that creates problems when pasting the content in spreadsheets.
+    # `file.info()$size` reports the size in bytes, hence `useBytes = TRUE`.
+    readChar(path, file.info(path)$size - 1L, useBytes = TRUE)
+}
+
+write_delim_impl <- function(x, path, delim, include_header) {
+    # Scope the `con` lifetime to just this helper.
+    # We need to `close()` the connection before we try and get the
+    # `file.info()$size`.
+
+    # Must open in binary write mode, otherwise even though we set
+    # `eol = "\n"` on Windows it will still write `\r\n` due to the C
+    # level `vfprintf()` call.
+    con <- file(path, open = "wb")
+    defer(close(con))
+
+    utils::write.table(
+      x = x,
+      file = con,
+      sep = delim,
+      eol = "\n",
+      row.names = FALSE,
+      col.names = include_header,
+      quote = FALSE,
+      na = ""
+    )
 }
 
 write_html <- function(x, include_header) {
@@ -415,7 +440,9 @@ profile_histogram <- function(x, method = c("fixed", "sturges", "fd", "scott"),
 
   # For dates, we convert back the breaks to the date representation.
   if (inherits(x, "POSIXct")) {
-    bin_edges <- as.POSIXct(h$breaks, tz = attr(x, "tzone"))
+    # Must supply an `origin` on R <= 4.2
+    origin <- as.POSIXct("1970-01-01", tz = "UTC")
+    bin_edges <- as.POSIXct(h$breaks, tz = attr(x, "tzone"), origin = origin)
   }
 
   list(

crates/ark/src/sys/windows.rs

@@ -8,6 +8,7 @@
 pub mod console;
 pub mod control;
 pub mod interface;
+mod locale;
 pub mod path;
 pub mod signals;
 mod strings;