From c9385fbb6b2e985d6c03034cf44eb05237c7e417 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= <deluan@navidrome.org>
Date: Mon, 31 Aug 2026 21:42:10 -0400
Subject: [PATCH] test(plugins): build test plugins in Go instead of shelling
 out to make (#6060)

* test(plugins): build test plugins in Go instead of shelling out to make

The plugins suite built its .ndp test packages by running `make -C
plugins/testdata`, which needs make and zip on the PATH. That is the reason
the 26 WASM-dependent spec files are tagged //go:build !windows.

buildTestPlugins now does the same work in Go: the same mtime check make
performed, `GOOS=wasip1 GOARCH=wasm go build` per plugin, and archive/zip
for the package. TinyGo was already optional and unused in CI, so nothing is
lost there. The first plugin builds on its own so the shared wasip1 stdlib
and PDK objects land in the build cache before the rest fan out: on a cold
cache that is 2.2s against 3.4s for the sequential make and 7.3s for an
unrestrained fan-out.

Packaging moved into a writeNdp helper shared with createTestPackage, which
was already writing the same two-entry archive. Entries are written in a
fixed order, so the .ndp bytes are now reproducible; the loader hashes those
bytes, and `zip` also stored file mtimes, so the previous packages differed
on every rebuild.

The Makefile is unchanged and still works for building the plugins by hand.
Removing the !windows tags is a separate step, once CI is green here.

* test(plugins): run the WASM plugin specs on Windows

With the test plugins now built in Go, nothing in the suite needs a Unix
toolchain, so the //go:build !windows tags come off all 25 spec files. The
Windows CI job runs `go test ./...`, so it picks the suite up with no
workflow change.

plugins_suite_windows_test.go existed only to bootstrap the handful of specs
that compiled on Windows; plugins_suite_test.go now serves both.

* test(plugins): skip the planted-symlink spec where symlinks need privileges

os.Symlink needs an elevated token or Developer Mode on Windows, so the
unconditional Expect(...).To(Succeed()) would fail for contributors running
the suite on an ordinary Windows box. The elevated GitHub runner hides this.

The equivalent spec in sandbox_fs_internal_test.go already attempts the
symlink and skips on error; this does the same, keeping the pin live
everywhere it can run, including Windows CI.

* fix(ci): stop the Windows ndpgen test failing silently

The ndpgen suite builds its helper binary to %TEMP%\ndpgen-test, and Windows
will not exec a file without an executable extension, so the "supports
verbose mode" spec has been failing there. Nobody noticed because the
Test ndpgen step ran under pwsh, which carries on after a non-zero exit and
takes the step's status from the last command, so the job stayed green with
a FAIL line in its log.

Add the .exe suffix, and run the step under bash like the Linux job does, so
a failure in any of its three commands fails the job.
---
 .github/workflows/pipeline.yml           |   6 +-
 plugins/cmd/ndpgen/integration_test.go   |   4 +
 plugins/host_artwork_test.go             |   2 -
 plugins/host_cache_test.go               |   2 -
 plugins/host_config_test.go              |   2 -
 plugins/host_httpclient_test.go          |   2 -
 plugins/host_kvstore_test.go             |   2 -
 plugins/host_library_test.go             |   2 -
 plugins/host_matcher_test.go             |   2 -
 plugins/host_scheduler_test.go           |   2 -
 plugins/host_scrobbleretriever_test.go   |   2 -
 plugins/host_storage_test.go             |   2 -
 plugins/host_subsonicapi_test.go         |   2 -
 plugins/host_taskqueue_test.go           |   2 -
 plugins/host_users_test.go               |   2 -
 plugins/host_websocket_test.go           |   2 -
 plugins/lyrics_adapter_test.go           |   2 -
 plugins/manager_call_test.go             |   2 -
 plugins/manager_loader_load_test.go      |   2 -
 plugins/manager_readonly_test.go         |   2 -
 plugins/manager_test.go                  |   2 -
 plugins/manager_watcher_test.go          |   2 -
 plugins/metadata_agent_test.go           |   2 -
 plugins/package_test.go                  |  50 +++++-----
 plugins/plugins_suite_test.go            |  12 ---
 plugins/plugins_suite_windows_test.go    |  23 -----
 plugins/sandbox_fs_test.go               |   6 +-
 plugins/scrobbler_adapter_test.go        |   2 -
 plugins/sonic_similarity_adapter_test.go |   2 -
 plugins/testdata_builder_test.go         | 119 +++++++++++++++++++++++
 30 files changed, 156 insertions(+), 110 deletions(-)
 delete mode 100644 plugins/plugins_suite_windows_test.go
 create mode 100644 plugins/testdata_builder_test.go

diff --git a/.github/workflows/pipeline.yml b/.github/workflows/pipeline.yml
index 9da99e864..37b532eb0 100644
--- a/.github/workflows/pipeline.yml
+++ b/.github/workflows/pipeline.yml
@@ -237,12 +237,12 @@ jobs:
         run: go test -shuffle=on -tags netgo,sqlite_fts5 ./... -v
 
       - name: Test ndpgen
-        shell: pwsh
+        shell: bash
         run: |
-          cd plugins\cmd\ndpgen
+          cd plugins/cmd/ndpgen
           go test -shuffle=on -v
           go build -o ndpgen.exe .
-          .\ndpgen.exe --help
+          ./ndpgen.exe --help
 
   js:
     name: Test JS code
diff --git a/plugins/cmd/ndpgen/integration_test.go b/plugins/cmd/ndpgen/integration_test.go
index d8bc5859d..bbe886f8d 100644
--- a/plugins/cmd/ndpgen/integration_test.go
+++ b/plugins/cmd/ndpgen/integration_test.go
@@ -6,6 +6,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"runtime"
 	"strings"
 
 	. "github.com/onsi/ginkgo/v2"
@@ -32,6 +33,9 @@ var _ = Describe("ndpgen CLI", Ordered, func() {
 
 		// Build the ndpgen binary
 		ndpgenBin = filepath.Join(os.TempDir(), "ndpgen-test")
+		if runtime.GOOS == "windows" {
+			ndpgenBin += ".exe" // Windows refuses to exec a file without it
+		}
 		cmd := exec.Command("go", "build", "-o", ndpgenBin, ".")
 		cmd.Dir = mustGetWd(GinkgoT())
 		output, err := cmd.CombinedOutput()
diff --git a/plugins/host_artwork_test.go b/plugins/host_artwork_test.go
index 50a1a0335..311bf21d5 100644
--- a/plugins/host_artwork_test.go
+++ b/plugins/host_artwork_test.go
@@ -1,5 +1,3 @@
-//go:build !windows
-
 package plugins
 
 import (
diff --git a/plugins/host_cache_test.go b/plugins/host_cache_test.go
index 5e91b04ec..925856701 100644
--- a/plugins/host_cache_test.go
+++ b/plugins/host_cache_test.go
@@ -1,5 +1,3 @@
-//go:build !windows
-
 package plugins
 
 import (
diff --git a/plugins/host_config_test.go b/plugins/host_config_test.go
index b296d29fb..80b06efe1 100644
--- a/plugins/host_config_test.go
+++ b/plugins/host_config_test.go
@@ -1,5 +1,3 @@
-//go:build !windows
-
 package plugins
 
 import (
diff --git a/plugins/host_httpclient_test.go b/plugins/host_httpclient_test.go
index 27e92d59d..4eb34a247 100644
--- a/plugins/host_httpclient_test.go
+++ b/plugins/host_httpclient_test.go
@@ -1,5 +1,3 @@
-//go:build !windows
-
 package plugins
 
 import (
diff --git a/plugins/host_kvstore_test.go b/plugins/host_kvstore_test.go
index 997409146..9af3fcbcd 100644
--- a/plugins/host_kvstore_test.go
+++ b/plugins/host_kvstore_test.go
@@ -1,5 +1,3 @@
-//go:build !windows
-
 package plugins
 
 import (
diff --git a/plugins/host_library_test.go b/plugins/host_library_test.go
index eb5b17a02..00a953b24 100644
--- a/plugins/host_library_test.go
+++ b/plugins/host_library_test.go
@@ -1,5 +1,3 @@
-//go:build !windows
-
 package plugins
 
 import (
diff --git a/plugins/host_matcher_test.go b/plugins/host_matcher_test.go
index 9d44ee581..f0967628c 100644
--- a/plugins/host_matcher_test.go
+++ b/plugins/host_matcher_test.go
@@ -1,5 +1,3 @@
-//go:build !windows
-
 package plugins
 
 import (
diff --git a/plugins/host_scheduler_test.go b/plugins/host_scheduler_test.go
index ca53aed56..6940d8820 100644
--- a/plugins/host_scheduler_test.go
+++ b/plugins/host_scheduler_test.go
@@ -1,5 +1,3 @@
-//go:build !windows
-
 package plugins
 
 import (
diff --git a/plugins/host_scrobbleretriever_test.go b/plugins/host_scrobbleretriever_test.go
index ab817c282..aa92c9eb5 100644
--- a/plugins/host_scrobbleretriever_test.go
+++ b/plugins/host_scrobbleretriever_test.go
@@ -1,5 +1,3 @@
-//go:build !windows
-
 package plugins
 
 import (
diff --git a/plugins/host_storage_test.go b/plugins/host_storage_test.go
index d6f895bff..9fc58c396 100644
--- a/plugins/host_storage_test.go
+++ b/plugins/host_storage_test.go
@@ -1,5 +1,3 @@
-//go:build !windows
-
 package plugins
 
 import (
diff --git a/plugins/host_subsonicapi_test.go b/plugins/host_subsonicapi_test.go
index c3d9ffe8a..4b941bc43 100644
--- a/plugins/host_subsonicapi_test.go
+++ b/plugins/host_subsonicapi_test.go
@@ -1,5 +1,3 @@
-//go:build !windows
-
 package plugins
 
 import (
diff --git a/plugins/host_taskqueue_test.go b/plugins/host_taskqueue_test.go
index d459fd69b..72fb4cccd 100644
--- a/plugins/host_taskqueue_test.go
+++ b/plugins/host_taskqueue_test.go
@@ -1,5 +1,3 @@
-//go:build !windows
-
 package plugins
 
 import (
diff --git a/plugins/host_users_test.go b/plugins/host_users_test.go
index 42f6a3032..1721d3ee2 100644
--- a/plugins/host_users_test.go
+++ b/plugins/host_users_test.go
@@ -1,5 +1,3 @@
-//go:build !windows
-
 package plugins
 
 import (
diff --git a/plugins/host_websocket_test.go b/plugins/host_websocket_test.go
index 2aa85cd21..9f2d20bef 100644
--- a/plugins/host_websocket_test.go
+++ b/plugins/host_websocket_test.go
@@ -1,5 +1,3 @@
-//go:build !windows
-
 package plugins
 
 import (
diff --git a/plugins/lyrics_adapter_test.go b/plugins/lyrics_adapter_test.go
index d110665f5..fe15e40b2 100644
--- a/plugins/lyrics_adapter_test.go
+++ b/plugins/lyrics_adapter_test.go
@@ -1,5 +1,3 @@
-//go:build !windows
-
 package plugins
 
 import (
diff --git a/plugins/manager_call_test.go b/plugins/manager_call_test.go
index 3e64f1cee..5e2c6385e 100644
--- a/plugins/manager_call_test.go
+++ b/plugins/manager_call_test.go
@@ -1,5 +1,3 @@
-//go:build !windows
-
 package plugins
 
 import (
diff --git a/plugins/manager_loader_load_test.go b/plugins/manager_loader_load_test.go
index 8f35548af..eb004276e 100644
--- a/plugins/manager_loader_load_test.go
+++ b/plugins/manager_loader_load_test.go
@@ -1,5 +1,3 @@
-//go:build !windows
-
 package plugins
 
 import (
diff --git a/plugins/manager_readonly_test.go b/plugins/manager_readonly_test.go
index 9d16b76e9..019b14fbf 100644
--- a/plugins/manager_readonly_test.go
+++ b/plugins/manager_readonly_test.go
@@ -1,5 +1,3 @@
-//go:build !windows
-
 package plugins
 
 import (
diff --git a/plugins/manager_test.go b/plugins/manager_test.go
index 9b6f7ea39..6cf90994a 100644
--- a/plugins/manager_test.go
+++ b/plugins/manager_test.go
@@ -1,5 +1,3 @@
-//go:build !windows
-
 package plugins
 
 import (
diff --git a/plugins/manager_watcher_test.go b/plugins/manager_watcher_test.go
index 5b5ffca02..99326bde1 100644
--- a/plugins/manager_watcher_test.go
+++ b/plugins/manager_watcher_test.go
@@ -1,5 +1,3 @@
-//go:build !windows
-
 package plugins
 
 import (
diff --git a/plugins/metadata_agent_test.go b/plugins/metadata_agent_test.go
index a7a0aa8b8..0a7b6c1e2 100644
--- a/plugins/metadata_agent_test.go
+++ b/plugins/metadata_agent_test.go
@@ -1,5 +1,3 @@
-//go:build !windows
-
 package plugins
 
 import (
diff --git a/plugins/package_test.go b/plugins/package_test.go
index 953797750..1539d8df7 100644
--- a/plugins/package_test.go
+++ b/plugins/package_test.go
@@ -2,8 +2,11 @@ package plugins
 
 import (
 	"archive/zip"
+	"bytes"
 	"encoding/json"
+	"errors"
 	"fmt"
+	"io"
 	"os"
 	"path/filepath"
 
@@ -266,6 +269,14 @@ func (h *testZipHelper) close() error {
 // createTestPackage creates an .ndp package file from a manifest and wasm bytes.
 // This is primarily used for testing.
 func createTestPackage(ndpPath string, manifest *Manifest, wasmBytes []byte) error {
+	manifestBytes, err := json.Marshal(manifest)
+	if err != nil {
+		return fmt.Errorf("marshaling manifest: %w", err)
+	}
+	return writeNdp(ndpPath, bytes.NewReader(manifestBytes), bytes.NewReader(wasmBytes))
+}
+
+func writeNdp(ndpPath string, manifest, wasm io.Reader) error {
 	f, err := os.Create(ndpPath)
 	if err != nil {
 		return fmt.Errorf("creating package file: %w", err)
@@ -273,30 +284,23 @@ func createTestPackage(ndpPath string, manifest *Manifest, wasmBytes []byte) err
 	defer f.Close()
 
 	zw := zip.NewWriter(f)
-	defer zw.Close()
-
-	// Write manifest.json
-	manifestBytes, err := json.Marshal(manifest)
-	if err != nil {
-		return fmt.Errorf("marshaling manifest: %w", err)
+	add := func(name string, r io.Reader) error {
+		w, err := zw.Create(name)
+		if err != nil {
+			return fmt.Errorf("creating %s in package: %w", name, err)
+		}
+		if _, err := io.Copy(w, r); err != nil {
+			return fmt.Errorf("writing %s: %w", name, err)
+		}
+		return nil
 	}
-
-	mw, err := zw.Create(manifestFileName)
-	if err != nil {
-		return fmt.Errorf("creating manifest in zip: %w", err)
+	// Entry order is fixed: the loader hashes the package bytes, so they must
+	// be reproducible across rebuilds.
+	if err := add(manifestFileName, manifest); err != nil {
+		return err
 	}
-	if _, err := mw.Write(manifestBytes); err != nil {
-		return fmt.Errorf("writing manifest: %w", err)
+	if err := add(wasmFileName, wasm); err != nil {
+		return err
 	}
-
-	// Write plugin.wasm
-	ww, err := zw.Create(wasmFileName)
-	if err != nil {
-		return fmt.Errorf("creating wasm in zip: %w", err)
-	}
-	if _, err := ww.Write(wasmBytes); err != nil {
-		return fmt.Errorf("writing wasm: %w", err)
-	}
-
-	return nil
+	return errors.Join(zw.Close(), f.Close())
 }
diff --git a/plugins/plugins_suite_test.go b/plugins/plugins_suite_test.go
index 2dae65d00..cc1b45f97 100644
--- a/plugins/plugins_suite_test.go
+++ b/plugins/plugins_suite_test.go
@@ -1,5 +1,3 @@
-//go:build !windows
-
 package plugins
 
 import (
@@ -7,15 +5,12 @@ import (
 	"crypto/sha256"
 	"encoding/hex"
 	"encoding/json"
-	"fmt"
 	"net/http"
 	"os"
-	"os/exec"
 	"path/filepath"
 	"runtime"
 	"strings"
 	"testing"
-	"time"
 
 	"github.com/navidrome/navidrome/conf"
 	"github.com/navidrome/navidrome/conf/configtest"
@@ -51,13 +46,6 @@ func TestPlugins(t *testing.T) {
 	RunSpecs(t, "Plugins Suite")
 }
 
-func buildTestPlugins(path string) {
-	start := time.Now()
-	out, err := exec.Command("make", "-C", path).CombinedOutput()
-	fmt.Fprintf(GinkgoWriter, "[BeforeSuite] built test plugins in %s:\n%s", time.Since(start), out)
-	Expect(err).ToNot(HaveOccurred(), "failed to build test plugins")
-}
-
 // createTestManager creates a new plugin Manager with the given plugin config.
 // It creates a temp directory, copies the test-metadata-agent plugin, and starts the manager.
 // Returns the manager, temp directory path, and a cleanup function.
diff --git a/plugins/plugins_suite_windows_test.go b/plugins/plugins_suite_windows_test.go
deleted file mode 100644
index ed43bdcc3..000000000
--- a/plugins/plugins_suite_windows_test.go
+++ /dev/null
@@ -1,23 +0,0 @@
-//go:build windows
-
-package plugins
-
-import (
-	"testing"
-
-	"github.com/navidrome/navidrome/log"
-	"github.com/navidrome/navidrome/tests"
-	. "github.com/onsi/ginkgo/v2"
-	. "github.com/onsi/gomega"
-)
-
-// Runs the subset of plugin specs compiled on Windows (files without the
-// //go:build !windows tag): capabilities, manager_cache, manager_plugin,
-// manifest, package. WASM-runtime-dependent specs live in !windows-tagged
-// files and aren't reached here.
-func TestPlugins(t *testing.T) {
-	tests.Init(t, false)
-	log.SetLevel(log.LevelFatal)
-	RegisterFailHandler(Fail)
-	RunSpecs(t, "Plugins Suite")
-}
diff --git a/plugins/sandbox_fs_test.go b/plugins/sandbox_fs_test.go
index ea033283f..0dd9d15ab 100644
--- a/plugins/sandbox_fs_test.go
+++ b/plugins/sandbox_fs_test.go
@@ -1,5 +1,3 @@
-//go:build !windows
-
 package plugins
 
 import (
@@ -134,7 +132,9 @@ var _ = Describe("Plugin filesystem sandbox", Ordered, ContinueOnFailure, func()
 
 	// Accepted residual, pinned so a future tightening can't happen silently
 	It("still follows a symlink planted in the mount by something else", func() {
-		Expect(os.Symlink(outsideDir, filepath.Join(libraryDir, "planted"))).To(Succeed())
+		if err := os.Symlink(outsideDir, filepath.Join(libraryDir, "planted")); err != nil {
+			Skip("cannot create symlinks here: " + err.Error()) // Windows without privileges
+		}
 
 		out := call(sandboxInput{Operation: "write_file", FilePath: "planted/via-symlink.txt", Content: "escaped"})
 
diff --git a/plugins/scrobbler_adapter_test.go b/plugins/scrobbler_adapter_test.go
index 3efd5d1b1..00c9045a4 100644
--- a/plugins/scrobbler_adapter_test.go
+++ b/plugins/scrobbler_adapter_test.go
@@ -1,5 +1,3 @@
-//go:build !windows
-
 package plugins
 
 import (
diff --git a/plugins/sonic_similarity_adapter_test.go b/plugins/sonic_similarity_adapter_test.go
index cb08b9acd..e08fc124f 100644
--- a/plugins/sonic_similarity_adapter_test.go
+++ b/plugins/sonic_similarity_adapter_test.go
@@ -1,5 +1,3 @@
-//go:build !windows
-
 package plugins
 
 import (
diff --git a/plugins/testdata_builder_test.go b/plugins/testdata_builder_test.go
new file mode 100644
index 000000000..746a8af22
--- /dev/null
+++ b/plugins/testdata_builder_test.go
@@ -0,0 +1,119 @@
+package plugins
+
+import (
+	"errors"
+	"fmt"
+	"io/fs"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"slices"
+	"sync"
+	"time"
+
+	"github.com/navidrome/navidrome/utils"
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+// buildTestPlugins packages every test plugin under dir, replicating
+// `make -C plugins/testdata` without needing make or zip on the PATH.
+func buildTestPlugins(dir string) {
+	start := time.Now()
+	built, err := buildPackages(dir)
+	fmt.Fprintf(GinkgoWriter, "[BeforeSuite] built test plugins in %s: %v\n", time.Since(start), built)
+	Expect(err).ToNot(HaveOccurred(), "failed to build test plugins")
+}
+
+func buildPackages(dir string) ([]string, error) {
+	mods, err := filepath.Glob(filepath.Join(dir, "*", "go.mod"))
+	if err != nil || len(mods) == 0 {
+		return nil, err
+	}
+	pdkTime, err := newestModTime(filepath.Join(dir, "..", "pdk", "go"))
+	if err != nil {
+		return nil, err
+	}
+
+	built := make([]string, len(mods))
+	errs := make([]error, len(mods))
+	build := func(i int) {
+		pluginDir := filepath.Dir(mods[i])
+		var rebuilt bool
+		if rebuilt, errs[i] = buildPackage(pluginDir, pdkTime); rebuilt {
+			built[i] = filepath.Base(pluginDir)
+		}
+	}
+
+	// The first build populates the wasip1 stdlib and PDK objects every plugin
+	// shares; fanning out before it lands makes each one compile them again.
+	build(0)
+	var wg sync.WaitGroup
+	for i := range mods[1:] {
+		wg.Go(func() { build(i + 1) })
+	}
+	wg.Wait()
+	return slices.DeleteFunc(built, func(name string) bool { return name == "" }), errors.Join(errs...)
+}
+
+func buildPackage(dir string, pdkTime time.Time) (bool, error) {
+	sourceTime, err := newestModTime(dir)
+	if err != nil {
+		return false, err
+	}
+	pkg := dir + PackageExtension
+	if info, err := os.Stat(pkg); err == nil && info.ModTime().After(utils.TimeNewest(sourceTime, pdkTime)) {
+		return false, nil
+	}
+
+	wasm, err := filepath.Abs(pkg + ".build.wasm")
+	if err != nil {
+		return false, err
+	}
+	defer os.Remove(wasm)
+	// -buildvcs=false keeps the bytes stable across commits, so the suite's
+	// wazero compilation cache still hits after a rebuild.
+	cmd := exec.Command("go", "build", "-buildvcs=false", "-buildmode=c-shared", "-o", wasm, ".")
+	cmd.Dir = dir
+	cmd.Env = append(os.Environ(), "GOOS=wasip1", "GOARCH=wasm")
+	if out, err := cmd.CombinedOutput(); err != nil {
+		return false, fmt.Errorf("building %s: %w\n%s", dir, err, out)
+	}
+
+	tmp := pkg + ".build.ndp"
+	defer os.Remove(tmp)
+	if err := packageFiles(tmp, filepath.Join(dir, manifestFileName), wasm); err != nil {
+		return false, fmt.Errorf("packaging %s: %w", dir, err)
+	}
+	return true, os.Rename(tmp, pkg)
+}
+
+func packageFiles(pkg, manifest, wasm string) error {
+	m, err := os.Open(manifest)
+	if err != nil {
+		return err
+	}
+	defer m.Close()
+	w, err := os.Open(wasm)
+	if err != nil {
+		return err
+	}
+	defer w.Close()
+	return writeNdp(pkg, m, w)
+}
+
+func newestModTime(root string) (time.Time, error) {
+	var newest time.Time
+	err := filepath.WalkDir(root, func(_ string, d fs.DirEntry, err error) error {
+		if err != nil || d.IsDir() {
+			return err
+		}
+		info, err := d.Info()
+		if err != nil {
+			return err
+		}
+		newest = utils.TimeNewest(newest, info.ModTime())
+		return nil
+	})
+	return newest, err
+}