From 1292b02aff9ed27c9fc449300530d2d478cf2d7c Mon Sep 17 00:00:00 2001 From: Deluan Date: Mon, 22 Dec 2025 13:44:16 -0500 Subject: [PATCH] tests(plugins): more optimizations Signed-off-by: Deluan --- .gitignore | 1 + Makefile | 2 +- plugins/manager_test.go | 79 +++++------ plugins/metadata_agent_test.go | 138 ++++++++++++------- plugins/plugins_suite_test.go | 70 ++++++++++ plugins/testdata/Makefile | 10 +- plugins/testdata/fake-metadata-agent/main.go | 43 ++++++ plugins/watcher_integration_test.go | 42 ++---- 8 files changed, 255 insertions(+), 130 deletions(-) diff --git a/.gitignore b/.gitignore index 03852f9ab..028664f47 100644 --- a/.gitignore +++ b/.gitignore @@ -17,6 +17,7 @@ master.zip testDB cache/* *.swp +coverage.out dist music *.db* diff --git a/Makefile b/Makefile index 45e43f73e..c2913a77c 100644 --- a/Makefile +++ b/Makefile @@ -50,7 +50,7 @@ test: ##@Development Run Go tests. Use PKG variable to specify packages to test, go test -tags netgo $(PKG) .PHONY: test -testall: test-race test-i18n test-js ##@Development Run Go and JS tests +testall: test test-i18n test-js ##@Development Run Go and JS tests .PHONY: testall test-race: ##@Development Run Go tests with race detector diff --git a/plugins/manager_test.go b/plugins/manager_test.go index ab7f83dcd..d0aae0936 100644 --- a/plugins/manager_test.go +++ b/plugins/manager_test.go @@ -3,65 +3,49 @@ package plugins import ( "context" "fmt" - "path/filepath" - "runtime" "sync" "github.com/navidrome/navidrome/conf" - "github.com/navidrome/navidrome/conf/configtest" "github.com/navidrome/navidrome/core/agents" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) var _ = Describe("Manager", Ordered, func() { - var ( - manager *Manager - ctx context.Context - testdataDir string - ) + var ctx context.Context + // Ensure plugin is loaded at the start (might have been unloaded by previous tests) BeforeAll(func() { - ctx = context.Background() - - // Get testdata directory (where fake-metadata-agent.wasm lives) - _, currentFile, _, ok := runtime.Caller(0) - Expect(ok).To(BeTrue()) - testdataDir = filepath.Join(filepath.Dir(currentFile), "testdata") - - // Setup config - DeferCleanup(configtest.SetupConfig()) - conf.Server.Plugins.Enabled = true - conf.Server.Plugins.Folder = testdataDir - - // Create manager once for all tests - manager = &Manager{ - plugins: make(map[string]*pluginInstance), + ctx = GinkgoT().Context() + if _, ok := testManager.plugins["fake-metadata-agent"]; !ok { + err := testManager.LoadPlugin("fake-metadata-agent") + Expect(err).ToNot(HaveOccurred()) } - err := manager.Start(ctx) - Expect(err).ToNot(HaveOccurred()) + }) - DeferCleanup(func() { - _ = manager.Stop() - }) + // Ensure plugin is restored after all tests in this block + AfterAll(func() { + if _, ok := testManager.plugins["fake-metadata-agent"]; !ok { + _ = testManager.LoadPlugin("fake-metadata-agent") + } }) Describe("LoadPlugin", func() { It("auto-loads plugins from folder on Start", func() { - // Plugin is already loaded by manager.Start() via discoverPlugins - names := manager.PluginNames(string(CapabilityMetadataAgent)) + // Plugin is already loaded by testManager.Start() via discoverPlugins + names := testManager.PluginNames(string(CapabilityMetadataAgent)) Expect(names).To(ContainElement("fake-metadata-agent")) }) It("returns error when plugin file does not exist", func() { - err := manager.LoadPlugin("nonexistent") + err := testManager.LoadPlugin("nonexistent") Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("plugin file not found")) }) It("returns error when plugin is already loaded", func() { // Plugin was loaded on Start, try to load again - err := manager.LoadPlugin("fake-metadata-agent") + err := testManager.LoadPlugin("fake-metadata-agent") Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("already loaded")) }) @@ -76,7 +60,7 @@ var _ = Describe("Manager", Ordered, func() { conf.Server.DataFolder = originalDataFolder }() - err := manager.LoadPlugin("test") + err := testManager.LoadPlugin("test") Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("no plugins folder configured")) }) @@ -85,24 +69,24 @@ var _ = Describe("Manager", Ordered, func() { Describe("UnloadPlugin", func() { It("removes a loaded plugin", func() { // Plugin is already loaded from Start - err := manager.UnloadPlugin("fake-metadata-agent") + err := testManager.UnloadPlugin("fake-metadata-agent") Expect(err).ToNot(HaveOccurred()) - names := manager.PluginNames(string(CapabilityMetadataAgent)) + names := testManager.PluginNames(string(CapabilityMetadataAgent)) Expect(names).ToNot(ContainElement("fake-metadata-agent")) }) It("can reload after unload", func() { // Reload the plugin we just unloaded - err := manager.LoadPlugin("fake-metadata-agent") + err := testManager.LoadPlugin("fake-metadata-agent") Expect(err).ToNot(HaveOccurred()) - names := manager.PluginNames(string(CapabilityMetadataAgent)) + names := testManager.PluginNames(string(CapabilityMetadataAgent)) Expect(names).To(ContainElement("fake-metadata-agent")) }) It("returns error when plugin not found", func() { - err := manager.UnloadPlugin("nonexistent") + err := testManager.UnloadPlugin("nonexistent") Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("not found")) }) @@ -110,24 +94,33 @@ var _ = Describe("Manager", Ordered, func() { Describe("ReloadPlugin", func() { It("unloads and reloads a plugin", func() { - err := manager.ReloadPlugin("fake-metadata-agent") + err := testManager.ReloadPlugin("fake-metadata-agent") Expect(err).ToNot(HaveOccurred()) - names := manager.PluginNames(string(CapabilityMetadataAgent)) + names := testManager.PluginNames(string(CapabilityMetadataAgent)) Expect(names).To(ContainElement("fake-metadata-agent")) }) It("returns error when plugin not found", func() { - err := manager.ReloadPlugin("nonexistent") + err := testManager.ReloadPlugin("nonexistent") Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("failed to unload")) }) }) + Describe("GetPluginInfo", func() { + It("returns information about all loaded plugins", func() { + info := testManager.GetPluginInfo() + Expect(info).To(HaveKey("fake-metadata-agent")) + Expect(info["fake-metadata-agent"].Name).To(Equal("Test Plugin")) + Expect(info["fake-metadata-agent"].Version).To(Equal("1.0.0")) + }) + }) + It("can call the plugin concurrently", func() { // Plugin is already loaded - const concurrency = 100 + const concurrency = 30 errs := make(chan error, concurrency) bios := make(chan string, concurrency) @@ -136,7 +129,7 @@ var _ = Describe("Manager", Ordered, func() { for i := range concurrency { go func(i int) { defer g.Done() - a, ok := manager.LoadMediaAgent("fake-metadata-agent") + a, ok := testManager.LoadMediaAgent("fake-metadata-agent") Expect(ok).To(BeTrue()) agent := a.(agents.ArtistBiographyRetriever) bio, err := agent.GetArtistBiography(ctx, fmt.Sprintf("artist-%d", i), fmt.Sprintf("Artist %d", i), "") diff --git a/plugins/metadata_agent_test.go b/plugins/metadata_agent_test.go index aeae47e63..2f72bc234 100644 --- a/plugins/metadata_agent_test.go +++ b/plugins/metadata_agent_test.go @@ -2,69 +2,22 @@ package plugins import ( "context" - "os" - "path/filepath" - "runtime" - "github.com/navidrome/navidrome/conf" - "github.com/navidrome/navidrome/conf/configtest" "github.com/navidrome/navidrome/core/agents" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) var _ = Describe("MetadataAgent", Ordered, func() { - var ( - manager *Manager - agent agents.Interface - ctx context.Context - testdataDir string - tmpDir string - ) + var agent agents.Interface + var ctx context.Context BeforeAll(func() { ctx = GinkgoT().Context() - - // Get testdata directory - _, currentFile, _, ok := runtime.Caller(0) + // Load the agent via shared manager + var ok bool + agent, ok = testManager.LoadMediaAgent("fake-metadata-agent") Expect(ok).To(BeTrue()) - testdataDir = filepath.Join(filepath.Dir(currentFile), "testdata") - - // Create temp dir for plugins - var err error - tmpDir, err = os.MkdirTemp("", "metadata-agent-test-*") - Expect(err).ToNot(HaveOccurred()) - - // Copy test plugin to temp dir - srcPath := filepath.Join(testdataDir, "fake-metadata-agent.wasm") - destPath := filepath.Join(tmpDir, "fake-metadata-agent.wasm") - data, err := os.ReadFile(srcPath) - Expect(err).ToNot(HaveOccurred()) - err = os.WriteFile(destPath, data, 0600) - Expect(err).ToNot(HaveOccurred()) - - // Setup config - DeferCleanup(configtest.SetupConfig()) - conf.Server.Plugins.Enabled = true - conf.Server.Plugins.Folder = tmpDir - conf.Server.CacheFolder = filepath.Join(tmpDir, "cache") - - // Create and start the manager - manager = &Manager{ - plugins: make(map[string]*pluginInstance), - } - err = manager.Start(ctx) - Expect(err).ToNot(HaveOccurred()) - - // Load the agent via manager - var ok2 bool - agent, ok2 = manager.LoadMediaAgent("fake-metadata-agent") - Expect(ok2).To(BeTrue()) - - DeferCleanup(func() { - _ = manager.Stop() - _ = os.RemoveAll(tmpDir) - }) }) Describe("AgentName", func() { @@ -160,3 +113,84 @@ var _ = Describe("MetadataAgent", Ordered, func() { }) }) }) + +var _ = Describe("MetadataAgent error handling", Ordered, func() { + // Tests error paths when plugin is configured to return errors + var ( + errorManager *Manager + errorAgent agents.Interface + ctx context.Context + ) + + BeforeAll(func() { + ctx = GinkgoT().Context() + + // Create manager with error injection config + errorManager, _ = createTestManager(map[string]map[string]string{ + "fake-metadata-agent": { + "error": "simulated plugin error", + }, + }) + + // Load the agent + var ok bool + errorAgent, ok = errorManager.LoadMediaAgent("fake-metadata-agent") + Expect(ok).To(BeTrue()) + }) + + It("returns error from GetArtistMBID", func() { + retriever := errorAgent.(agents.ArtistMBIDRetriever) + _, err := retriever.GetArtistMBID(ctx, "artist-1", "Test") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("simulated plugin error")) + }) + + It("returns error from GetArtistURL", func() { + retriever := errorAgent.(agents.ArtistURLRetriever) + _, err := retriever.GetArtistURL(ctx, "artist-1", "Test", "mbid") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("simulated plugin error")) + }) + + It("returns error from GetArtistBiography", func() { + retriever := errorAgent.(agents.ArtistBiographyRetriever) + _, err := retriever.GetArtistBiography(ctx, "artist-1", "Test", "mbid") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("simulated plugin error")) + }) + + It("returns error from GetArtistImages", func() { + retriever := errorAgent.(agents.ArtistImageRetriever) + _, err := retriever.GetArtistImages(ctx, "artist-1", "Test", "mbid") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("simulated plugin error")) + }) + + It("returns error from GetSimilarArtists", func() { + retriever := errorAgent.(agents.ArtistSimilarRetriever) + _, err := retriever.GetSimilarArtists(ctx, "artist-1", "Test", "mbid", 5) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("simulated plugin error")) + }) + + It("returns error from GetArtistTopSongs", func() { + retriever := errorAgent.(agents.ArtistTopSongsRetriever) + _, err := retriever.GetArtistTopSongs(ctx, "artist-1", "Test", "mbid", 5) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("simulated plugin error")) + }) + + It("returns error from GetAlbumInfo", func() { + retriever := errorAgent.(agents.AlbumInfoRetriever) + _, err := retriever.GetAlbumInfo(ctx, "Album", "Artist", "mbid") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("simulated plugin error")) + }) + + It("returns error from GetAlbumImages", func() { + retriever := errorAgent.(agents.AlbumImageRetriever) + _, err := retriever.GetAlbumImages(ctx, "Album", "Artist", "mbid") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("simulated plugin error")) + }) +}) diff --git a/plugins/plugins_suite_test.go b/plugins/plugins_suite_test.go index 6533ff713..7caae405e 100644 --- a/plugins/plugins_suite_test.go +++ b/plugins/plugins_suite_test.go @@ -3,9 +3,14 @@ package plugins import ( + "os" "os/exec" + "path/filepath" + "runtime" "testing" + "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/conf/configtest" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/tests" . "github.com/onsi/ginkgo/v2" @@ -14,6 +19,13 @@ import ( const testDataDir = "plugins/testdata" +// Shared test state initialized in BeforeSuite +var ( + testdataDir string // Path to testdata folder with fake-metadata-agent.wasm + tmpPluginsDir string // Temp directory for plugin tests that modify files + testManager *Manager +) + func TestPlugins(t *testing.T) { tests.Init(t, false) buildTestPlugins(t, testDataDir) @@ -32,3 +44,61 @@ func buildTestPlugins(t *testing.T, path string) { t.Fatalf("Failed to build test plugins: %v", err) } } + +// createTestManager creates a new plugin Manager with the given plugin config. +// It creates a temp directory, copies the fake plugin, and starts the manager. +// Returns the manager, temp directory path, and a cleanup function. +func createTestManager(pluginConfig map[string]map[string]string) (*Manager, string) { + // Create temp directory + tmpDir, err := os.MkdirTemp("", "plugins-test-*") + Expect(err).ToNot(HaveOccurred()) + + // Copy test plugin to temp dir + srcPath := filepath.Join(testdataDir, "fake-metadata-agent.wasm") + destPath := filepath.Join(tmpDir, "fake-metadata-agent.wasm") + data, err := os.ReadFile(srcPath) + Expect(err).ToNot(HaveOccurred()) + err = os.WriteFile(destPath, data, 0600) + Expect(err).ToNot(HaveOccurred()) + + // Setup config + DeferCleanup(configtest.SetupConfig()) + conf.Server.Plugins.Enabled = true + conf.Server.Plugins.Folder = tmpDir + conf.Server.Plugins.AutoReload = false + conf.Server.CacheFolder = filepath.Join(tmpDir, "cache") + conf.Server.PluginConfig = pluginConfig + + // Create and start manager + manager := &Manager{ + plugins: make(map[string]*pluginInstance), + } + err = manager.Start(GinkgoT().Context()) + Expect(err).ToNot(HaveOccurred()) + + DeferCleanup(func() { + _ = manager.Stop() + _ = os.RemoveAll(tmpDir) + }) + + return manager, tmpDir +} + +var _ = BeforeSuite(func() { + // Get testdata directory (where fake-metadata-agent.wasm lives) + _, currentFile, _, ok := runtime.Caller(0) + Expect(ok).To(BeTrue()) + testdataDir = filepath.Join(filepath.Dir(currentFile), "testdata") + + // Create shared manager for most tests + testManager, tmpPluginsDir = createTestManager(nil) +}) + +var _ = AfterSuite(func() { + if testManager != nil { + _ = testManager.Stop() + } + if tmpPluginsDir != "" { + _ = os.RemoveAll(tmpPluginsDir) + } +}) diff --git a/plugins/testdata/Makefile b/plugins/testdata/Makefile index ecc7139b8..9c7baaa1b 100644 --- a/plugins/testdata/Makefile +++ b/plugins/testdata/Makefile @@ -2,10 +2,18 @@ # Auto-discover all plugin folders (folders containing go.mod) PLUGINS := $(patsubst %/go.mod,%,$(wildcard */go.mod)) +# Prefer tinygo if available, it produces smaller wasm binaries and +# makes the tests faster. +TINYGO := $(shell command -v tinygo 2> /dev/null) + all: $(PLUGINS:%=%.wasm) clean: rm -f $(PLUGINS:%=%.wasm) %.wasm: %/main.go %/go.mod - cd $* && GOOS=wasip1 GOARCH=wasm go build -buildmode=c-shared -o ../$@ . \ No newline at end of file +ifdef TINYGO + cd $* && tinygo build -target wasip1 -buildmode=c-shared -o ../$@ . +else + cd $* && GOOS=wasip1 GOARCH=wasm go build -buildmode=c-shared -o ../$@ . +endif \ No newline at end of file diff --git a/plugins/testdata/fake-metadata-agent/main.go b/plugins/testdata/fake-metadata-agent/main.go index f163037f7..db34edbac 100644 --- a/plugins/testdata/fake-metadata-agent/main.go +++ b/plugins/testdata/fake-metadata-agent/main.go @@ -4,10 +4,29 @@ package main import ( "encoding/json" + "strconv" "github.com/extism/go-pdk" ) +// checkConfigError checks if the plugin is configured to return an error. +// If "error" config is set, it returns the error message and exit code. +// If "exitcode" is also set, it uses that value (default: 1). +func checkConfigError() (bool, int32) { + errMsg, hasErr := pdk.GetConfig("error") + if !hasErr || errMsg == "" { + return false, 0 + } + exitCode := int32(1) + if code, hasCode := pdk.GetConfig("exitcode"); hasCode { + if parsed, err := strconv.Atoi(code); err == nil { + exitCode = int32(parsed) + } + } + pdk.SetErrorString(errMsg) + return true, exitCode +} + type Manifest struct { Name string `json:"name"` Author string `json:"author"` @@ -130,6 +149,9 @@ func ndManifest() int32 { //go:wasmexport nd_get_artist_mbid func ndGetArtistMBID() int32 { + if hasErr, code := checkConfigError(); hasErr { + return code + } var input ArtistInput if err := pdk.InputJSON(&input); err != nil { pdk.SetError(err) @@ -145,6 +167,9 @@ func ndGetArtistMBID() int32 { //go:wasmexport nd_get_artist_url func ndGetArtistURL() int32 { + if hasErr, code := checkConfigError(); hasErr { + return code + } var input ArtistInput if err := pdk.InputJSON(&input); err != nil { pdk.SetError(err) @@ -160,6 +185,9 @@ func ndGetArtistURL() int32 { //go:wasmexport nd_get_artist_biography func ndGetArtistBiography() int32 { + if hasErr, code := checkConfigError(); hasErr { + return code + } var input ArtistInput if err := pdk.InputJSON(&input); err != nil { pdk.SetError(err) @@ -175,6 +203,9 @@ func ndGetArtistBiography() int32 { //go:wasmexport nd_get_artist_images func ndGetArtistImages() int32 { + if hasErr, code := checkConfigError(); hasErr { + return code + } var input ArtistInput if err := pdk.InputJSON(&input); err != nil { pdk.SetError(err) @@ -195,6 +226,9 @@ func ndGetArtistImages() int32 { //go:wasmexport nd_get_similar_artists func ndGetSimilarArtists() int32 { + if hasErr, code := checkConfigError(); hasErr { + return code + } var input ArtistInputWithLimit if err := pdk.InputJSON(&input); err != nil { pdk.SetError(err) @@ -220,6 +254,9 @@ func ndGetSimilarArtists() int32 { //go:wasmexport nd_get_artist_top_songs func ndGetArtistTopSongs() int32 { + if hasErr, code := checkConfigError(); hasErr { + return code + } var input ArtistInputWithCount if err := pdk.InputJSON(&input); err != nil { pdk.SetError(err) @@ -245,6 +282,9 @@ func ndGetArtistTopSongs() int32 { //go:wasmexport nd_get_album_info func ndGetAlbumInfo() int32 { + if hasErr, code := checkConfigError(); hasErr { + return code + } var input AlbumInput if err := pdk.InputJSON(&input); err != nil { pdk.SetError(err) @@ -265,6 +305,9 @@ func ndGetAlbumInfo() int32 { //go:wasmexport nd_get_album_images func ndGetAlbumImages() int32 { + if hasErr, code := checkConfigError(); hasErr { + return code + } var input AlbumInput if err := pdk.InputJSON(&input); err != nil { pdk.SetError(err) diff --git a/plugins/watcher_integration_test.go b/plugins/watcher_integration_test.go index 1f5d9c486..a5952cd42 100644 --- a/plugins/watcher_integration_test.go +++ b/plugins/watcher_integration_test.go @@ -4,58 +4,34 @@ import ( "context" "os" "path/filepath" - "runtime" "testing" "github.com/navidrome/navidrome/conf" - "github.com/navidrome/navidrome/conf/configtest" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/rjeczalik/notify" ) var _ = Describe("Watcher Integration", Ordered, func() { + // Uses testdataDir and createTestManager from BeforeSuite var ( - manager *Manager - ctx context.Context - testdataDir string - tmpDir string + manager *Manager + tmpDir string + ctx context.Context ) BeforeAll(func() { if testing.Short() { Skip("Skipping integration test in short mode") } - ctx = GinkgoT().Context() - // Get testdata directory - _, currentFile, _, ok := runtime.Caller(0) - Expect(ok).To(BeTrue()) - testdataDir = filepath.Join(filepath.Dir(currentFile), "testdata") + // Create manager for watcher lifecycle tests (no plugin preloaded - tests copy plugin as needed) + manager, tmpDir = createTestManager(nil) - // Create temp dir for plugins - var err error - tmpDir, err = os.MkdirTemp("", "plugins-watcher-integration-*") - Expect(err).ToNot(HaveOccurred()) - - // Setup config (AutoReload disabled - tests inject events directly) - DeferCleanup(configtest.SetupConfig()) - conf.Server.Plugins.Enabled = true - conf.Server.Plugins.Folder = tmpDir - conf.Server.Plugins.AutoReload = false - - // Create a fresh manager for each test - manager = &Manager{ - plugins: make(map[string]*pluginInstance), - } - err = manager.Start(ctx) - Expect(err).ToNot(HaveOccurred()) - - DeferCleanup(func() { - _ = manager.Stop() - _ = os.RemoveAll(tmpDir) - }) + // Remove the auto-loaded plugin so tests can control loading + _ = manager.UnloadPlugin("fake-metadata-agent") + _ = os.Remove(filepath.Join(tmpDir, "fake-metadata-agent.wasm")) }) // Helper to copy test plugin into the temp folder