Fix replaygain paths for mp3gain/aacgain on Windows / fix tests (#6373)

## PR Summary

Fixes #2946
Fixes #6335

Replaygain plugin tests used a **fragmented configuration approach**:
each backend mixin set only `backend` as a class variable, while
additional `command` configuration was required for `CmdBackendMixin`.

### Key changes

- **`CmdBackendMixin`** now explicitly sets `"command": "mp3gain"`
alongside the backend name.
- All tests now reference plugin-scoped `self.config` configuration
instead of global one.
- The `GAIN_PROG_AVAILABLE` check now also looks for `mp3rgain` in
addition to `mp3gain` and `aacgain`.
- `mp3gain` is now installed for both `ubuntu` and `windows`-based
runners.
- Tests on Windows caught a legitimate issue #2946 which this PR fixes
also.

This was missed out in #6289 since these tests did not run in CI.

.github/workflows/ci.yaml

@@ -35,7 +35,12 @@ jobs:
           python-version: ${{ matrix.python-version }}
           cache: poetry
 
-      - name: Install PyGobject and release script dependencies on Ubuntu
+      - name: Install system dependencies on Windows
+        if: matrix.platform == 'windows-latest'
+        run: |
+          choco install mp3gain -y
+
+      - name: Install system dependencies on Ubuntu
         if: matrix.platform == 'ubuntu-latest'
         run: |
           sudo apt update
@@ -43,11 +48,12 @@ jobs:
             ffmpeg \
             gobject-introspection \
             gstreamer1.0-plugins-base \
-            python3-gst-1.0 \
+            imagemagick \
             libcairo2-dev \
             libgirepository-2.0-dev \
+            mp3gain \
             pandoc \
-            imagemagick
+            python3-gst-1.0
 
       - name: Get changed lyrics files
         id: lyrics-update

beetsplug/replaygain.py

@@ -34,7 +34,7 @@ from typing import TYPE_CHECKING, Any, ClassVar, Literal, TypeVar
 
 from beets import ui
 from beets.plugins import BeetsPlugin
-from beets.util import command_output, displayable_path, syspath
+from beets.util import command_output, syspath
 
 if TYPE_CHECKING:
     import optparse
@@ -425,7 +425,7 @@ class FfmpegBackend(Backend):
         # call ffmpeg
         self._log.debug("analyzing {}", item)
         cmd = self._construct_cmd(item, peak_method)
-        self._log.debug("executing {}", " ".join(map(displayable_path, cmd)))
+        self._log.debug("executing {}", " ".join(cmd))
         output = call(cmd, self._log).stderr.splitlines()
 
         # parse output
@@ -643,18 +643,20 @@ class CommandBackend(Backend):
         # tag-writing; this turns the mp3gain/aacgain tool into a gain
         # calculator rather than a tag manipulator because we take care
         # of changing tags ourselves.
-        cmd: list[str] = [self.command, "-o", "-s", "s"]
-        if self.noclip:
-            # Adjust to avoid clipping.
-            cmd = [*cmd, "-k"]
-        else:
-            # Disable clipping warning.
-            cmd = [*cmd, "-c"]
-        cmd = [*cmd, "-d", str(int(target_level - 89))]
-        cmd = cmd + [syspath(i.path) for i in items]
+        cmd = [
+            self.command,
+            "-o",
+            "-s",
+            "s",
+            # Avoid clipping or disable clipping warning
+            "-k" if self.noclip else "-c",
+            "-d",
+            str(int(target_level - 89)),
+            *[str(i.filepath) for i in items],
+        ]
 
         self._log.debug("analyzing {} files", len(items))
-        self._log.debug("executing {}", " ".join(map(displayable_path, cmd)))
+        self._log.debug("executing {}", " ".join(cmd))
         output = call(cmd, self._log).stdout
         self._log.debug("analysis finished")
         return self.parse_tool_output(

docs/changelog.rst

@@ -24,6 +24,8 @@ Bug fixes
 - :doc:`plugins/musicbrainz`: Fix fetching very large releases that have more
   than 500 tracks. :bug:`6355`
 - :doc:`plugins/badfiles`: Fix number of found errors in log message
+- :doc:`plugins/replaygain`: Avoid magic Windows prefix in calls to command
+  backends, such as ``mp3gain``. :bug:`2946`
 
 ..
     For plugin developers

test/plugins/test_replaygain.py

@@ -14,12 +14,11 @@
 
 
 import unittest
-from typing import ClassVar
+from typing import Any, ClassVar
 
 import pytest
 from mediafile import MediaFile
 
-from beets import config
 from beets.test.helper import (
     AsIsImporterMixin,
     ImportTestCase,
@@ -39,10 +38,15 @@ try:
 except (ImportError, ValueError):
     GST_AVAILABLE = False
 
-if any(has_program(cmd, ["-v"]) for cmd in ["mp3gain", "aacgain"]):
-    GAIN_PROG_AVAILABLE = True
-else:
-    GAIN_PROG_AVAILABLE = False
+
+GAIN_PROG = next(
+    (
+        cmd
+        for cmd in ["mp3gain", "mp3rgain", "aacgain"]
+        if has_program(cmd, ["-v"])
+    ),
+    None,
+)
 
 FFMPEG_AVAILABLE = has_program("ffmpeg", ["-version"])
 
@@ -63,14 +67,18 @@ class ReplayGainTestCase(PluginMixin, ImportTestCase):
     plugin = "replaygain"
     preload_plugin = False
 
-    backend: ClassVar[str]
+    plugin_config: ClassVar[dict[str, Any]]
+
+    @property
+    def backend(self):
+        return self.plugin_config["backend"]
 
     def setUp(self):
         # Implemented by Mixins, see above. This may decide to skip the test.
         self.test_backend()
 
         super().setUp()
-        self.config["replaygain"]["backend"] = self.backend
+        self.config["replaygain"].set(self.plugin_config)
 
         self.load_plugins()
 
@@ -81,8 +89,16 @@ class ThreadedImportMixin:
         self.config["threaded"] = True
 
 
-class GstBackendMixin:
-    backend = "gstreamer"
+class BackendMixin:
+    plugin_config: ClassVar[dict[str, Any]]
+    has_r128_support: bool
+
+    def test_backend(self):
+        """Check whether the backend actually has all required functionality."""
+
+
+class GstBackendMixin(BackendMixin):
+    plugin_config: ClassVar[dict[str, Any]] = {"backend": "gstreamer"}
     has_r128_support = True
 
     def test_backend(self):
@@ -90,30 +106,25 @@ class GstBackendMixin:
         try:
             # Check if required plugins can be loaded by instantiating a
             # GStreamerBackend (via its .__init__).
-            config["replaygain"]["targetlevel"] = 89
-            GStreamerBackend(config["replaygain"], None)
+            self.config["replaygain"]["targetlevel"] = 89
+            GStreamerBackend(self.config["replaygain"], None)
         except FatalGstreamerPluginReplayGainError as e:
             # Skip the test if plugins could not be loaded.
             self.skipTest(str(e))
 
 
-class CmdBackendMixin:
-    backend = "command"
+class CmdBackendMixin(BackendMixin):
+    plugin_config: ClassVar[dict[str, Any]] = {
+        "backend": "command",
+        "command": GAIN_PROG,
+    }
     has_r128_support = False
 
-    def test_backend(self):
-        """Check whether the backend actually has all required functionality."""
-        pass
 
-
-class FfmpegBackendMixin:
-    backend = "ffmpeg"
+class FfmpegBackendMixin(BackendMixin):
+    plugin_config: ClassVar[dict[str, Any]] = {"backend": "ffmpeg"}
     has_r128_support = True
 
-    def test_backend(self):
-        """Check whether the backend actually has all required functionality."""
-        pass
-
 
 class ReplayGainCliTest:
     FNAME: str
@@ -332,7 +343,7 @@ class ReplayGainGstCliTest(
     FNAME = "full"  # file contains only silence
 
 
-@unittest.skipIf(not GAIN_PROG_AVAILABLE, "no *gain command found")
+@unittest.skipIf(not GAIN_PROG, "no *gain command found")
 class ReplayGainCmdCliTest(
     ReplayGainCliTest, ReplayGainTestCase, CmdBackendMixin
 ):
@@ -369,7 +380,7 @@ class ReplayGainGstImportTest(ImportTest, ReplayGainTestCase, GstBackendMixin):
     pass
 
 
-@unittest.skipIf(not GAIN_PROG_AVAILABLE, "no *gain command found")
+@unittest.skipIf(not GAIN_PROG, "no *gain command found")
 class ReplayGainCmdImportTest(ImportTest, ReplayGainTestCase, CmdBackendMixin):
     pass