diff options
| -rwxr-xr-x | scripts/normalize-notify-sounds.sh | 26 | ||||
| -rw-r--r-- | tests/normalize-notify/fake-ffmpeg | 25 | ||||
| -rw-r--r-- | tests/normalize-notify/test_normalize_notify_sounds.py | 117 |
3 files changed, 162 insertions, 6 deletions
diff --git a/scripts/normalize-notify-sounds.sh b/scripts/normalize-notify-sounds.sh index 72c4c33..7dffbbc 100755 --- a/scripts/normalize-notify-sounds.sh +++ b/scripts/normalize-notify-sounds.sh @@ -28,6 +28,12 @@ shopt -s nullglob files=("$SOUND_DIR"/*.ogg) (( ${#files[@]} )) || { echo "No .ogg files in $SOUND_DIR" >&2; exit 1; } +# Clean up the in-flight temp file on any exit (a failed encode under set -e +# aborts mid-loop, so the trap is what stops the temp from leaking). +tmp="" +cleanup() { [ -n "${tmp:-}" ] && rm -f "$tmp"; return 0; } +trap cleanup EXIT + for f in "${files[@]}"; do mean=$(ffmpeg -hide_banner -nostats -i "$f" -af volumedetect -f null /dev/null 2>&1 \ | grep -oP 'mean_volume: \K[-0-9.]+' || true) @@ -36,14 +42,22 @@ for f in "${files[@]}"; do continue fi gain=$(awk -v t="$TARGET_DB" -v m="$mean" 'BEGIN { printf "%.1f", t - m }') - tmp=$(mktemp --suffix=.ogg) + # Resolve the real target (SOUND_DIR is often the stow-symlinked ~/.local + # copy) and stage the temp beside it, so the mv is atomic on the same + # filesystem and replacing the real file leaves the stow symlink pointing + # at it. A truncate-in-place (cat > "$f") would have corrupted the tracked + # file if the encode produced a short or empty output. + target=$(readlink -f "$f") + tmp=$(mktemp --suffix=.ogg --tmpdir="$(dirname "$target")") ffmpeg -hide_banner -loglevel error -y -i "$f" \ -af "volume=${gain}dB" -c:a libvorbis -q:a 6 "$tmp" - # Write through the file rather than mv over it: when SOUND_DIR is the - # stow-symlinked ~/.local copy, mv would replace the symlink with a real - # file and decouple it from the repo. cat preserves the symlink target. - cat "$tmp" > "$f" - rm -f "$tmp" + if [ ! -s "$tmp" ]; then + echo "skip (empty re-encode): $f" >&2 + rm -f "$tmp"; tmp="" + continue + fi + mv -f "$tmp" "$target" + tmp="" printf "%-14s mean %7s dB gain %+6s dB -> target %s dB\n" \ "$(basename "$f")" "$mean" "$gain" "$TARGET_DB" done diff --git a/tests/normalize-notify/fake-ffmpeg b/tests/normalize-notify/fake-ffmpeg new file mode 100644 index 0000000..cdfcb6f --- /dev/null +++ b/tests/normalize-notify/fake-ffmpeg @@ -0,0 +1,25 @@ +#!/bin/bash +# Fake ffmpeg for the normalize-notify-sounds tests. Two modes: +# measure (args include the `volumedetect` filter): print a mean_volume line +# to stderr, driven by FAKE_MEAN (default -20.0). +# encode (otherwise): write to the output file (the last argument), driven +# by FAKE_FFMPEG_EMPTY (1 -> zero-byte output) and FAKE_FFMPEG_FAIL +# (1 -> exit non-zero without writing). +set -uo pipefail + +for a in "$@"; do + if [ "$a" = "volumedetect" ]; then + echo "mean_volume: ${FAKE_MEAN:--20.0} dB" >&2 + exit 0 + fi +done + +out="${*: -1}" +if [ "${FAKE_FFMPEG_FAIL:-0}" = 1 ]; then + exit 1 +fi +if [ "${FAKE_FFMPEG_EMPTY:-0}" = 1 ]; then + : > "$out" +else + echo ENCODED > "$out" +fi diff --git a/tests/normalize-notify/test_normalize_notify_sounds.py b/tests/normalize-notify/test_normalize_notify_sounds.py new file mode 100644 index 0000000..ce3084b --- /dev/null +++ b/tests/normalize-notify/test_normalize_notify_sounds.py @@ -0,0 +1,117 @@ +"""Tests for normalize-notify-sounds.sh temp handling and atomic write. + +The script re-encodes each .ogg in place. SOUND_DIR is often the stow-symlinked +~/.local copy, so the write must land on the real repo file and preserve the +symlink. Two bugs the fix addresses: + +- A failed or empty re-encode used to truncate the target with `cat "$tmp" > + "$f"`, corrupting a repo-tracked sound to zero bytes. +- The mktemp had no EXIT trap, so an interrupted encode leaked a temp file. + +ffmpeg/ffprobe are faked via stubs on PATH (this directory) so the encode +result is controllable without real audio work. + +Run from repo root: + python3 -m unittest tests.normalize-notify.test_normalize_notify_sounds +""" + +import glob +import os +import shutil +import subprocess +import tempfile +import unittest + +HERE = os.path.dirname(os.path.abspath(__file__)) +REPO_ROOT = os.path.abspath(os.path.join(HERE, "..", "..")) +SCRIPT = os.path.join(REPO_ROOT, "scripts", "normalize-notify-sounds.sh") + + +class NormalizeNotifySounds(unittest.TestCase): + def setUp(self): + self.tmp = tempfile.mkdtemp(prefix="norm-notify-test-") + self.addCleanup(shutil.rmtree, self.tmp, ignore_errors=True) + self.bindir = os.path.join(self.tmp, "bin") + os.mkdir(self.bindir) + for tool in ("ffmpeg", "ffprobe"): + fake = os.path.join(HERE, "fake-" + tool) + if not os.path.exists(fake): + fake = os.path.join(HERE, "fake-ffmpeg") # ffprobe reuses stub + dst = os.path.join(self.bindir, tool) + shutil.copy(fake, dst) + os.chmod(dst, 0o755) + # sound dir with one .ogg, plus a "repo" the sound symlinks into + self.repo = os.path.join(self.tmp, "repo") + os.mkdir(self.repo) + self.sounddir = os.path.join(self.tmp, "sounds") + os.mkdir(self.sounddir) + + def run_script(self, env_extra=None): + env = dict(os.environ) + env["PATH"] = self.bindir + os.pathsep + env["PATH"] + if env_extra: + env.update(env_extra) + return subprocess.run( + ["bash", SCRIPT, self.sounddir], + capture_output=True, text=True, timeout=20, env=env, + ) + + def make_sound(self, name="chime.ogg", body="ORIGINAL\n", symlink=False): + if symlink: + real = os.path.join(self.repo, name) + with open(real, "w") as f: + f.write(body) + link = os.path.join(self.sounddir, name) + os.symlink(real, link) + return link, real + path = os.path.join(self.sounddir, name) + with open(path, "w") as f: + f.write(body) + return path, path + + def leftover_temps(self, directory): + return glob.glob(os.path.join(directory, "*.ogg.*")) + \ + [p for p in glob.glob(os.path.join(directory, "tmp*")) if p.endswith(".ogg")] + + # --- Normal ---------------------------------------------------------- + + def test_reencodes_regular_file(self): + path, _ = self.make_sound() + r = self.run_script() + self.assertEqual(r.returncode, 0, r.stderr) + with open(path) as f: + self.assertEqual(f.read(), "ENCODED\n") + + def test_preserves_symlink_and_updates_target(self): + link, real = self.make_sound(symlink=True) + r = self.run_script() + self.assertEqual(r.returncode, 0, r.stderr) + self.assertTrue(os.path.islink(link), "the stow symlink must survive") + with open(real) as f: + self.assertEqual(f.read(), "ENCODED\n", + "the re-encode must land on the real repo file") + + # --- Error / corruption guard --------------------------------------- + + def test_empty_reencode_does_not_corrupt_target(self): + path, _ = self.make_sound(body="ORIGINAL\n") + self.run_script(env_extra={"FAKE_FFMPEG_EMPTY": "1"}) + with open(path) as f: + self.assertEqual(f.read(), "ORIGINAL\n", + "an empty re-encode must not truncate the tracked file") + + def test_failed_encode_leaves_no_temp_in_target_dir(self): + self.make_sound(symlink=True) + self.run_script(env_extra={"FAKE_FFMPEG_FAIL": "1"}) + self.assertEqual(self.leftover_temps(self.repo), [], + "a failed encode must not leak a temp file") + + def test_failed_encode_does_not_corrupt_target(self): + _, real = self.make_sound(symlink=True, body="ORIGINAL\n") + self.run_script(env_extra={"FAKE_FFMPEG_FAIL": "1"}) + with open(real) as f: + self.assertEqual(f.read(), "ORIGINAL\n") + + +if __name__ == "__main__": + unittest.main() |