Compare commits
26
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
2926ecd4b4 | ||
|
|
ff3c4003cb | ||
|
|
def596a99e | ||
|
|
14f78c0b57 | ||
|
|
7cea238e71 | ||
|
|
4f2f1827ab | ||
|
|
e454e4074b | ||
|
|
c518ac8c73 | ||
|
|
977f624123 | ||
|
|
23f3d4b3b0 | ||
|
|
8d46c4abb7 | ||
|
|
f714fe513d | ||
|
|
087c69ac8d | ||
|
|
bb7dde1963 | ||
|
|
446380e3a9 | ||
|
|
e07f248cc8 | ||
|
|
90ac6e0825 | ||
|
|
4e3c953acf | ||
|
|
ede183d026 | ||
|
|
481c9dca65 | ||
|
|
4025106234 | ||
|
|
a3134f997f | ||
|
|
3607fe445e | ||
|
|
61d549a9d5 | ||
|
|
2b84bc53e9 | ||
|
|
282dab43eb |
@@ -3664,3 +3664,35 @@ knowing before someone "fixes" it as broken: sampled from screenshots at
|
|||||||
900×600, the main panel's background goes 33,37,41 → 18,20,23 and a
|
900×600, the main panel's background goes 33,37,41 → 18,20,23 and a
|
||||||
row's text 242 → 133. It covers the content area only — not the sidebar
|
row's text 242 → 133. It covers the content area only — not the sidebar
|
||||||
or the transport — because the queue is not modal.
|
or the transport — because the queue is not modal.
|
||||||
|
|
||||||
|
## No test tier can see a `hover:` media query (measured 2026-08-19)
|
||||||
|
|
||||||
|
Gating an affordance on `(hover: hover) and (pointer: fine)` — #68's fix
|
||||||
|
for the play button that flashed on a long-press — is invisible to both
|
||||||
|
browser tiers, in *different* ways, and neither of them fails.
|
||||||
|
|
||||||
|
- **`make ui-test`**: CDP's `Emulation.setEmulatedMedia` with a `hover`
|
||||||
|
feature does not reach the tier's iframe. The call succeeds and
|
||||||
|
`matchMedia('(hover: hover)')` still answers `true` afterwards. So
|
||||||
|
there is no way to render a component as a phone would and read the
|
||||||
|
computed style.
|
||||||
|
- **`make e2e`**: both projects are desktop (`Desktop Chrome`,
|
||||||
|
`Desktop Safari`), and the phone specs reach phone *width* with
|
||||||
|
`setViewportSize`, which changes no media feature but `width`. So the
|
||||||
|
phone specs run with `hover: hover` and the gate is never exercised.
|
||||||
|
|
||||||
|
What does work, and what the fix was verified with, is a second browser
|
||||||
|
context under a device descriptor: `chromium.newContext(devices['Pixel
|
||||||
|
5'])` reports `hover=false pointer:fine=false` and the button computes
|
||||||
|
`display: none`, against `flex` at 1440px. That is a one-off script, not
|
||||||
|
a spec — `isMobile` is Chromium-only, so it cannot become an e2e project
|
||||||
|
without losing the WebKit half.
|
||||||
|
|
||||||
|
`hover-affordance.test.ts` therefore asserts the *parsed stylesheet* —
|
||||||
|
that the reveal rule sits inside the media query — which catches the
|
||||||
|
regression that actually threatens it: someone hoisting the rule back out
|
||||||
|
as a tidy-up, a change nothing on a desktop renders differently.
|
||||||
|
|
||||||
|
Related: a width-gated decision **is** testable at both tiers, which is
|
||||||
|
why #61's phone mini player is a `matchMedia` stub in the component test
|
||||||
|
and needs nothing special.
|
||||||
|
|||||||
@@ -2369,6 +2369,23 @@ Pre-commit hooks verify generated code is fresh — always run `make generate` a
|
|||||||
mistyped `feat` ships a minor version. `make release-dry` answers "what
|
mistyped `feat` ships a minor version. `make release-dry` answers "what
|
||||||
would this merge release" without pushing.
|
would this merge release" without pushing.
|
||||||
|
|
||||||
|
**The analyzer reads the type and ignores the scope, so a CI-only change
|
||||||
|
is `ci:` and never `fix(ci):`.** The scope is decoration; `fix` is a
|
||||||
|
patch whatever is in the brackets. Two commits touching nothing but
|
||||||
|
`.gitea/workflows/unclaim.yml` were written `fix(ci):` and cut `v0.2.1`
|
||||||
|
and `v0.2.2` — real releases, published to Arch, Homebrew and the APK
|
||||||
|
registry, containing no user-facing change. They were left in place
|
||||||
|
rather than deleted, because a version that vanishes is worse for
|
||||||
|
whoever pulled it than one that turns out to be empty.
|
||||||
|
|
||||||
|
**The blast radius is bigger than the version number**, which is what
|
||||||
|
makes this worth a paragraph. A merge to `main` starts two workflows;
|
||||||
|
if `release.yml` then pushes a tag, that tag push starts **four more**
|
||||||
|
(`arch-package`, `homebrew-formula`, `android-apk`, `desktop-assets`) —
|
||||||
|
on a runner with capacity 1, where the APK build alone is tens of
|
||||||
|
minutes. `make release-dry` before merging is how you find out, and it
|
||||||
|
is cheaper than every one of those.
|
||||||
|
|
||||||
**`@semantic-release/github` is not in that config and must not be.**
|
**`@semantic-release/github` is not in that config and must not be.**
|
||||||
Gitea's API is `/api/v1` and is not GitHub's surface, so
|
Gitea's API is `/api/v1` and is not GitHub's surface, so
|
||||||
`@semantic-release/exec` calls `scripts/gitea-release.sh` instead — one
|
`@semantic-release/exec` calls `scripts/gitea-release.sh` instead — one
|
||||||
|
|||||||
@@ -82,6 +82,100 @@ func TestPruneStaleLocalCrossReferences(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestPruneClearsInLibraryWithNoLocalID covers the fixed point: a row
|
||||||
|
// carrying in_library with a NULL local_*_id. The upsert's conflict
|
||||||
|
// clause is `in_library = MAX(in_library, excluded.in_library)`, so it
|
||||||
|
// can only ever raise the flag, and this pass used to be gated on the id
|
||||||
|
// being present — which meant nothing in the app could clear such a row,
|
||||||
|
// ever. It is asserted for all three entity types because the gate was
|
||||||
|
// written once and used three times, so a fix applied to one is a fix
|
||||||
|
// that looks complete.
|
||||||
|
//
|
||||||
|
// The rows are seeded with raw SQL rather than through seedIndexResult
|
||||||
|
// deliberately: upsertBatch writes a zero LocalArtistID as literal 0,
|
||||||
|
// not NULL, and 0 satisfies `IS NOT NULL` — so the old gate already
|
||||||
|
// caught that shape and a fixture built through the upsert cannot
|
||||||
|
// reproduce this at all. NULL is what the artifact importer and any
|
||||||
|
// older writer leave behind, the column being nullable with no default.
|
||||||
|
func TestPruneClearsInLibraryWithNoLocalID(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
db := database.NewTestDB(t)
|
||||||
|
si := NewSearchIndex(db, nil, nil, slog.Default())
|
||||||
|
|
||||||
|
// A genuinely owned artist, to prove the wider gate does not simply
|
||||||
|
// clear everything it now looks at.
|
||||||
|
database.InsertTestTrack(t, db, database.TestTrack{
|
||||||
|
FilePath: "/music/owned.mp3",
|
||||||
|
Artist: "Owned",
|
||||||
|
})
|
||||||
|
|
||||||
|
artist, err := db.Queries.GetArtistByName(t.Context(), "Owned")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("read seeded artist: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
seedIndexResult(t, db, SearchIndexResult{
|
||||||
|
EntityType: EntityArtist,
|
||||||
|
MBID: testMBID("owned"),
|
||||||
|
Title: "Owned",
|
||||||
|
ArtistName: "Owned",
|
||||||
|
ArtistMBID: testMBID("owned"),
|
||||||
|
InLibrary: true,
|
||||||
|
LocalArtistID: artist.ID,
|
||||||
|
})
|
||||||
|
|
||||||
|
orphans := []struct {
|
||||||
|
name string
|
||||||
|
entityType string
|
||||||
|
mbid string
|
||||||
|
}{
|
||||||
|
{"artist", EntityArtist, "orphan-artist"},
|
||||||
|
{"release group", EntityReleaseGroup, "orphan-release-group"},
|
||||||
|
{"recording", EntityRecording, "orphan-recording"},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, o := range orphans {
|
||||||
|
if _, err := db.ExecContext(
|
||||||
|
`INSERT INTO explore_index
|
||||||
|
(entity_type, mbid, title, artist_name, artist_mbid,
|
||||||
|
in_library,
|
||||||
|
local_artist_id, local_release_group_id, local_recording_id)
|
||||||
|
VALUES (?, ?, ?, ?, ?, 1, ?, ?, ?)`,
|
||||||
|
dbEntityType(o.entityType), dbMBID(testMBID(o.mbid)), o.name, o.name,
|
||||||
|
dbMBID(testMBID(o.mbid)),
|
||||||
|
nil, nil, nil,
|
||||||
|
); err != nil {
|
||||||
|
t.Fatalf("seed %s orphan: %v", o.name, err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
si.pruneStaleLocalCrossReferences()
|
||||||
|
|
||||||
|
inLibrary := func(t *testing.T, mbid string) int {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
var flag int
|
||||||
|
if err := db.QueryRowWriter(
|
||||||
|
"SELECT in_library FROM explore_index WHERE mbid = ?", dbMBID(mbid),
|
||||||
|
).Scan(&flag); err != nil {
|
||||||
|
t.Fatalf("read in_library for %q: %v", mbid, err)
|
||||||
|
}
|
||||||
|
|
||||||
|
return flag
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, o := range orphans {
|
||||||
|
if got := inLibrary(t, testMBID(o.mbid)); got != 0 {
|
||||||
|
t.Errorf("%s with a NULL local id: in_library = %d, want 0", o.name, got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if got := inLibrary(t, testMBID("owned")); got != 1 {
|
||||||
|
t.Errorf("owned artist: in_library = %d, want 1 (it still has a file)", got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// TestUnenrichedLibraryArtistMBIDs_OrdersByOwnedTrackCount verifies the
|
// TestUnenrichedLibraryArtistMBIDs_OrdersByOwnedTrackCount verifies the
|
||||||
// backfill queue prioritizes artists by how many tracks the user actually
|
// backfill queue prioritizes artists by how many tracks the user actually
|
||||||
// owns, not by how many duplicate-mbid artist rows happen to exist (the
|
// owns, not by how many duplicate-mbid artist rows happen to exist (the
|
||||||
|
|||||||
@@ -2562,6 +2562,19 @@ func (si *SearchIndex) PopulateLocalCrossReferences() {
|
|||||||
// The row itself is left in place (it may still be part of the shipped
|
// The row itself is left in place (it may still be part of the shipped
|
||||||
// catalog, just no longer owned) — only the "this is mine" bookkeeping
|
// catalog, just no longer owned) — only the "this is mine" bookkeeping
|
||||||
// is cleared.
|
// is cleared.
|
||||||
|
//
|
||||||
|
// It is gated on the flag *or* the id, not on the id alone. Gated on
|
||||||
|
// the id, `in_library = 1 AND local_*_id IS NULL` is a fixed point: the
|
||||||
|
// upsert can only ever raise the flag and this pass skipped such a row
|
||||||
|
// by construction, so nothing in the app could clear it — a row claiming
|
||||||
|
// to be owned, permanently, with no local row to check the claim
|
||||||
|
// against. Nothing in the tree writes that shape today
|
||||||
|
// (collectLibraryEntities sets both together), which is exactly why it
|
||||||
|
// is worth closing now: the exposure is a database written by an older
|
||||||
|
// version, and the next writer that sets the flag without an id, which
|
||||||
|
// nothing structurally prevents. A NULL id fails the existence test on
|
||||||
|
// its own, so the wider gate needs no second clause to say what "not
|
||||||
|
// owned" means.
|
||||||
func (si *SearchIndex) pruneStaleLocalCrossReferences() {
|
func (si *SearchIndex) pruneStaleLocalCrossReferences() {
|
||||||
type prune struct {
|
type prune struct {
|
||||||
entityType string
|
entityType string
|
||||||
@@ -2594,7 +2607,8 @@ func (si *SearchIndex) pruneStaleLocalCrossReferences() {
|
|||||||
result, err := si.db.ExecContext(
|
result, err := si.db.ExecContext(
|
||||||
`UPDATE explore_index
|
`UPDATE explore_index
|
||||||
SET in_library = 0, `+p.column+` = NULL
|
SET in_library = 0, `+p.column+` = NULL
|
||||||
WHERE entity_type = ? AND `+p.column+` IS NOT NULL
|
WHERE entity_type = ?
|
||||||
|
AND (`+p.column+` IS NOT NULL OR in_library = 1)
|
||||||
AND NOT EXISTS (`+p.exists+`)`,
|
AND NOT EXISTS (`+p.exists+`)`,
|
||||||
dbEntityType(p.entityType),
|
dbEntityType(p.entityType),
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -0,0 +1,188 @@
|
|||||||
|
package player
|
||||||
|
|
||||||
|
import (
|
||||||
|
"errors"
|
||||||
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
|
"github.com/gopxl/beep/v2"
|
||||||
|
)
|
||||||
|
|
||||||
|
// errTestDecode stands in for a decoder blowing up mid-track.
|
||||||
|
var errTestDecode = errors.New("decode blew up")
|
||||||
|
|
||||||
|
// stalledStreamer never produces a sample and never reports
|
||||||
|
// end-of-stream: (0, true), forever. A damaged file that decodes to
|
||||||
|
// nothing looks like this, and so does any source whose producer has
|
||||||
|
// quietly stopped.
|
||||||
|
type stalledStreamer struct{}
|
||||||
|
|
||||||
|
func (stalledStreamer) Stream(_ [][2]float64) (int, bool) { return 0, true }
|
||||||
|
func (stalledStreamer) Err() error { return nil }
|
||||||
|
|
||||||
|
// failingStreamer produces n good samples and then fails, which is
|
||||||
|
// what a decode error mid-track looks like: the same (0, false) a
|
||||||
|
// finished track returns, distinguishable only by Err.
|
||||||
|
type failingStreamer struct {
|
||||||
|
remaining int
|
||||||
|
err error
|
||||||
|
}
|
||||||
|
|
||||||
|
func (f *failingStreamer) Stream(samples [][2]float64) (int, bool) {
|
||||||
|
if f.remaining <= 0 {
|
||||||
|
return 0, false
|
||||||
|
}
|
||||||
|
|
||||||
|
n := min(len(samples), f.remaining)
|
||||||
|
|
||||||
|
for i := range n {
|
||||||
|
samples[i] = [2]float64{1, 1}
|
||||||
|
}
|
||||||
|
|
||||||
|
f.remaining -= n
|
||||||
|
|
||||||
|
return n, true
|
||||||
|
}
|
||||||
|
|
||||||
|
func (f *failingStreamer) Err() error { return f.err }
|
||||||
|
|
||||||
|
// drainUntilEnd calls Stream until it reports end-of-stream, or gives
|
||||||
|
// up. It returns whether the stream ended.
|
||||||
|
//
|
||||||
|
// The give-up bound is wall clock rather than a call count: the stall
|
||||||
|
// budget is a duration, so a tight loop has to actually wait it out.
|
||||||
|
func drainUntilEnd(bs *BufferedStreamer, within time.Duration) bool {
|
||||||
|
buf := make([][2]float64, 512)
|
||||||
|
deadline := time.Now().Add(within)
|
||||||
|
|
||||||
|
for time.Now().Before(deadline) {
|
||||||
|
if _, ok := bs.Stream(buf); !ok {
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
|
||||||
|
time.Sleep(time.Millisecond)
|
||||||
|
}
|
||||||
|
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
|
||||||
|
// A source that stops producing without ever ending is the fault this
|
||||||
|
// whole file exists for: Stream used to answer with silence and ok
|
||||||
|
// forever, so the chain never ended, the player stayed in Playing
|
||||||
|
// with the button showing pause, and the decoder's position never
|
||||||
|
// moved -- a frozen seek bar over a track that was not playing.
|
||||||
|
func TestAStalledSourceEndsTheStream(t *testing.T) {
|
||||||
|
bs := NewBufferedStreamer(stalledStreamer{}, 2048)
|
||||||
|
defer bs.Close()
|
||||||
|
|
||||||
|
if !drainUntilEnd(bs, maxStarvedDuration+2*time.Second) {
|
||||||
|
t.Fatal(
|
||||||
|
"a stalled source never ended the stream: the player " +
|
||||||
|
"would sit in Playing with a frozen position",
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
if !errors.Is(bs.Err(), errSourceStalled) {
|
||||||
|
t.Fatalf(
|
||||||
|
"expected the stall to be reported, got %v", bs.Err(),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Close is the other exit that used to leave `done` false, with the
|
||||||
|
// same consequence: the ring drains and every call after it is
|
||||||
|
// silence that claims to be audio.
|
||||||
|
func TestClosingEndsTheStream(t *testing.T) {
|
||||||
|
bs := NewBufferedStreamer(finiteStreamer(1<<20), 2048)
|
||||||
|
|
||||||
|
// Let the read-ahead fill something, so this exercises the drain
|
||||||
|
// after Close rather than a buffer that was empty anyway.
|
||||||
|
time.Sleep(20 * time.Millisecond)
|
||||||
|
bs.Close()
|
||||||
|
|
||||||
|
if !drainUntilEnd(bs, 2*time.Second) {
|
||||||
|
t.Fatal("a closed streamer never reported end-of-stream")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// A source that fails is not a source that finished, and only Err
|
||||||
|
// tells them apart. Before this, the player reported a mid-track
|
||||||
|
// decode failure to the queue as a natural end, so the queue
|
||||||
|
// auto-advanced in silence and counted the broken track as played.
|
||||||
|
func TestAFailedSourceReportsItsError(t *testing.T) {
|
||||||
|
src := &failingStreamer{remaining: 4096, err: errTestDecode}
|
||||||
|
|
||||||
|
bs := NewBufferedStreamer(src, 2048)
|
||||||
|
defer bs.Close()
|
||||||
|
|
||||||
|
if !drainUntilEnd(bs, 2*time.Second) {
|
||||||
|
t.Fatal("a failing source never reported end-of-stream")
|
||||||
|
}
|
||||||
|
|
||||||
|
if !errors.Is(bs.Err(), errTestDecode) {
|
||||||
|
t.Fatalf(
|
||||||
|
"expected the source's error to survive, got %v",
|
||||||
|
bs.Err(),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// The ordinary case has to keep working: a source that ends cleanly
|
||||||
|
// ends with no error, or every finished track would be reported as a
|
||||||
|
// failure and skipped.
|
||||||
|
func TestADrainedSourceReportsNoError(t *testing.T) {
|
||||||
|
bs := NewBufferedStreamer(finiteStreamer(4096), 2048)
|
||||||
|
defer bs.Close()
|
||||||
|
|
||||||
|
if !drainUntilEnd(bs, 2*time.Second) {
|
||||||
|
t.Fatal("a finite source never reported end-of-stream")
|
||||||
|
}
|
||||||
|
|
||||||
|
if bs.Err() != nil {
|
||||||
|
t.Fatalf(
|
||||||
|
"a track that finished normally reported %v", bs.Err(),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// A slow source is exactly what the read-ahead exists to absorb, so
|
||||||
|
// underruns must not be charged cumulatively -- otherwise a file on a
|
||||||
|
// slow disk ends itself partway through.
|
||||||
|
func TestUnderrunsDoNotAccumulateAcrossASlowSource(t *testing.T) {
|
||||||
|
const total = 8192
|
||||||
|
|
||||||
|
src := &slowStreamer{
|
||||||
|
inner: finiteStreamer(total),
|
||||||
|
delay: 2 * time.Millisecond,
|
||||||
|
}
|
||||||
|
|
||||||
|
bs := NewBufferedStreamer(src, 1024)
|
||||||
|
defer bs.Close()
|
||||||
|
|
||||||
|
buf := make([][2]float64, 256)
|
||||||
|
got := 0
|
||||||
|
|
||||||
|
for {
|
||||||
|
n, ok := bs.Stream(buf)
|
||||||
|
if !ok {
|
||||||
|
break
|
||||||
|
}
|
||||||
|
|
||||||
|
for i := range n {
|
||||||
|
if buf[i][0] != 0 {
|
||||||
|
got++
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if got != total {
|
||||||
|
t.Fatalf(
|
||||||
|
"a slow but healthy source was cut short: got %d of %d "+
|
||||||
|
"samples",
|
||||||
|
got, total,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// beep.Streamer is what the player wraps; keep the type honest.
|
||||||
|
var _ beep.Streamer = (*BufferedStreamer)(nil)
|
||||||
@@ -1,6 +1,7 @@
|
|||||||
package player
|
package player
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"errors"
|
||||||
"sync"
|
"sync"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
@@ -34,8 +35,45 @@ type BufferedStreamer struct {
|
|||||||
done bool
|
done bool
|
||||||
err error
|
err error
|
||||||
closed chan struct{}
|
closed chan struct{}
|
||||||
|
|
||||||
|
// starved counts consecutive Stream calls served with silence
|
||||||
|
// because the ring was empty, and starvedSince is when that run
|
||||||
|
// began. An underrun is legitimate for a moment -- that is what
|
||||||
|
// the read-ahead exists to absorb -- but it is not legitimate
|
||||||
|
// forever, and "forever" is indistinguishable from healthy
|
||||||
|
// playback everywhere above this type: the chain never ends, so
|
||||||
|
// the player stays in Playing with the button showing pause, and
|
||||||
|
// the decoder's position never moves, so the 1 Hz report pins the
|
||||||
|
// seek bar and suppresses its interpolation.
|
||||||
|
starved int
|
||||||
|
starvedSince time.Time
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// The silence fill is bounded by both a duration and a run of calls,
|
||||||
|
// and it needs both.
|
||||||
|
//
|
||||||
|
// Duration alone is the real measure -- the speaker paces itself, so
|
||||||
|
// wall clock is what says whether the source has actually stopped --
|
||||||
|
// but a caller draining in a tight loop (a test, a decode-to-buffer)
|
||||||
|
// makes hundreds of calls in microseconds and would trip nothing.
|
||||||
|
// A call count alone is the opposite failure: the same tight loop
|
||||||
|
// spends the whole budget before the read-ahead goroutine has been
|
||||||
|
// scheduled once, and ends a perfectly good stream at sample zero.
|
||||||
|
//
|
||||||
|
// The duration is longer than the 2 s read-ahead it is there to
|
||||||
|
// outlast, and the count is short enough that the speaker (~200 ms a
|
||||||
|
// call) reaches it well inside that.
|
||||||
|
const (
|
||||||
|
maxStarvedDuration = 3 * time.Second
|
||||||
|
minStarvedCalls = 8
|
||||||
|
)
|
||||||
|
|
||||||
|
// errSourceStalled is returned by Err when the source stopped
|
||||||
|
// producing samples without ever reporting end-of-stream.
|
||||||
|
var errSourceStalled = errors.New(
|
||||||
|
"audio source stopped producing samples",
|
||||||
|
)
|
||||||
|
|
||||||
// NewBufferedStreamer creates a BufferedStreamer that pre-fills
|
// NewBufferedStreamer creates a BufferedStreamer that pre-fills
|
||||||
// bufferSize samples from source via a background goroutine.
|
// bufferSize samples from source via a background goroutine.
|
||||||
// A typical bufferSize is 2× the sample rate (~2 seconds of audio).
|
// A typical bufferSize is 2× the sample rate (~2 seconds of audio).
|
||||||
@@ -54,8 +92,24 @@ func NewBufferedStreamer(
|
|||||||
return bs
|
return bs
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// finish marks the stream ended, recording err as the reason when
|
||||||
|
// there is one. Every exit from readAhead goes through it: an exit
|
||||||
|
// that leaves done false strands Stream in its underrun branch,
|
||||||
|
// where it returns silence and ok forever.
|
||||||
|
func (bs *BufferedStreamer) finish(err error) {
|
||||||
|
bs.mu.Lock()
|
||||||
|
defer bs.mu.Unlock()
|
||||||
|
|
||||||
|
bs.done = true
|
||||||
|
|
||||||
|
if err != nil && bs.err == nil {
|
||||||
|
bs.err = err
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// readAhead continuously reads from the source into the ring buffer
|
// readAhead continuously reads from the source into the ring buffer
|
||||||
// until the source is drained, an error occurs, or Close is called.
|
// until the source is drained, an error occurs, or Close is called.
|
||||||
|
// It always marks the stream done on the way out.
|
||||||
func (bs *BufferedStreamer) readAhead() {
|
func (bs *BufferedStreamer) readAhead() {
|
||||||
// Temporary buffer for reading from source outside the lock.
|
// Temporary buffer for reading from source outside the lock.
|
||||||
// 512 samples per chunk keeps the critical section short.
|
// 512 samples per chunk keeps the critical section short.
|
||||||
@@ -63,6 +117,13 @@ func (bs *BufferedStreamer) readAhead() {
|
|||||||
|
|
||||||
tmp := make([][2]float64, chunkSize)
|
tmp := make([][2]float64, chunkSize)
|
||||||
|
|
||||||
|
// Every exit marks the stream done. An exit that does not is what
|
||||||
|
// stranded Stream in its underrun branch, returning silence and ok
|
||||||
|
// for the rest of the process's life.
|
||||||
|
var exitErr error
|
||||||
|
|
||||||
|
defer func() { bs.finish(exitErr) }()
|
||||||
|
|
||||||
for {
|
for {
|
||||||
// Check if closed.
|
// Check if closed.
|
||||||
select {
|
select {
|
||||||
@@ -72,6 +133,15 @@ func (bs *BufferedStreamer) readAhead() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
bs.mu.Lock()
|
bs.mu.Lock()
|
||||||
|
|
||||||
|
// Stream gave up waiting for us. Nothing downstream is
|
||||||
|
// listening any more, so filling the ring is work for nobody.
|
||||||
|
if bs.done {
|
||||||
|
bs.mu.Unlock()
|
||||||
|
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
space := len(bs.ring) - bs.count
|
space := len(bs.ring) - bs.count
|
||||||
|
|
||||||
if space == 0 {
|
if space == 0 {
|
||||||
@@ -115,14 +185,12 @@ func (bs *BufferedStreamer) readAhead() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
if !ok {
|
if !ok {
|
||||||
bs.mu.Lock()
|
// A drained source and a failed one both land here and are
|
||||||
bs.done = true
|
// not the same event: one is a track that ended, the other
|
||||||
|
// is a track that broke. Err is what tells them apart, and
|
||||||
if srcErr := bs.source.Err(); srcErr != nil {
|
// it is why the player must ask before treating this as a
|
||||||
bs.err = srcErr
|
// natural finish.
|
||||||
}
|
exitErr = bs.source.Err()
|
||||||
|
|
||||||
bs.mu.Unlock()
|
|
||||||
|
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
@@ -154,7 +222,27 @@ func (bs *BufferedStreamer) Stream(
|
|||||||
}
|
}
|
||||||
|
|
||||||
if bs.count == 0 {
|
if bs.count == 0 {
|
||||||
// Buffer temporarily empty — fill with silence.
|
// The read-ahead has not caught up. Silence buys it time --
|
||||||
|
// but only for a bounded stretch, because "forever" is
|
||||||
|
// reported upward as healthy playback and there is no watchdog
|
||||||
|
// above this to notice otherwise.
|
||||||
|
bs.starved++
|
||||||
|
|
||||||
|
if bs.starvedSince.IsZero() {
|
||||||
|
bs.starvedSince = time.Now()
|
||||||
|
}
|
||||||
|
|
||||||
|
if bs.starved >= minStarvedCalls &&
|
||||||
|
time.Since(bs.starvedSince) > maxStarvedDuration {
|
||||||
|
bs.done = true
|
||||||
|
|
||||||
|
if bs.err == nil {
|
||||||
|
bs.err = errSourceStalled
|
||||||
|
}
|
||||||
|
|
||||||
|
return 0, false
|
||||||
|
}
|
||||||
|
|
||||||
for i := range samples {
|
for i := range samples {
|
||||||
samples[i] = [2]float64{}
|
samples[i] = [2]float64{}
|
||||||
}
|
}
|
||||||
@@ -162,6 +250,9 @@ func (bs *BufferedStreamer) Stream(
|
|||||||
return len(samples), true
|
return len(samples), true
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Samples arrived, so whatever the stall was, it is over.
|
||||||
|
bs.resetStarvationLocked()
|
||||||
|
|
||||||
// Copy available samples from ring buffer.
|
// Copy available samples from ring buffer.
|
||||||
n := len(samples)
|
n := len(samples)
|
||||||
if n > bs.count {
|
if n > bs.count {
|
||||||
@@ -197,6 +288,19 @@ func (bs *BufferedStreamer) Flush() {
|
|||||||
bs.readPos = 0
|
bs.readPos = 0
|
||||||
bs.writPos = 0
|
bs.writPos = 0
|
||||||
bs.count = 0
|
bs.count = 0
|
||||||
|
|
||||||
|
// A seek empties the ring on purpose, and the refill that follows
|
||||||
|
// is exactly the stall the budget exists to tolerate. Charging it
|
||||||
|
// against a budget the previous underrun already spent would end
|
||||||
|
// the track on a seek near the end of a slow file.
|
||||||
|
bs.resetStarvationLocked()
|
||||||
|
}
|
||||||
|
|
||||||
|
// resetStarvationLocked forgets an underrun run. Must be called with
|
||||||
|
// bs.mu held.
|
||||||
|
func (bs *BufferedStreamer) resetStarvationLocked() {
|
||||||
|
bs.starved = 0
|
||||||
|
bs.starvedSince = time.Time{}
|
||||||
}
|
}
|
||||||
|
|
||||||
// LockSource blocks the read-ahead goroutine from touching the
|
// LockSource blocks the read-ahead goroutine from touching the
|
||||||
|
|||||||
@@ -0,0 +1,213 @@
|
|||||||
|
package player
|
||||||
|
|
||||||
|
import (
|
||||||
|
"log/slog"
|
||||||
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
|
"github.com/wailsapp/wails/v3/pkg/application"
|
||||||
|
|
||||||
|
"yellowjacket/backend/events"
|
||||||
|
"yellowjacket/internal/testfixtures"
|
||||||
|
)
|
||||||
|
|
||||||
|
// fixtureSampleRate is what cmd/gentestdata writes (audio.go). It is
|
||||||
|
// deliberately not the speaker rate, which is what lets these tests
|
||||||
|
// tell the decoder's format from the player's default.
|
||||||
|
const fixtureSampleRate = 22050
|
||||||
|
|
||||||
|
// newTestPlayer is a player with a context and no database, so the
|
||||||
|
// track-metadata lookup cannot succeed.
|
||||||
|
func newTestPlayer(t *testing.T) *Player {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
p := NewPlayer(slog.Default(), nil)
|
||||||
|
rec := events.NewRecorder()
|
||||||
|
|
||||||
|
_ = p.ServiceStartup(
|
||||||
|
events.WithSink(t.Context(), rec),
|
||||||
|
application.ServiceOptions{},
|
||||||
|
)
|
||||||
|
|
||||||
|
return p
|
||||||
|
}
|
||||||
|
|
||||||
|
// loadFileLocked needs no speaker: it decodes, builds the chain and
|
||||||
|
// registers it paused. speaker.Play on an uninitialised device is
|
||||||
|
// what the integration guard elsewhere is about, so these assert on
|
||||||
|
// the state the load computed rather than on playback.
|
||||||
|
|
||||||
|
// p.format used to be assigned once, in the constructor, to the
|
||||||
|
// *speaker's* rate -- so it claimed 44.1 kHz for every file ever
|
||||||
|
// loaded. Play()'s replay-after-finish path resamples from it, so a
|
||||||
|
// finished track played again was resampled from a rate the decoder
|
||||||
|
// never produced: audibly the wrong speed and pitch, and wrong
|
||||||
|
// length and position arithmetic with it.
|
||||||
|
//
|
||||||
|
// The fixtures are 22050 Hz, which is exactly the point -- any of
|
||||||
|
// them disagrees with the speaker rate.
|
||||||
|
func TestLoadRecordsTheDecodersOwnFormat(t *testing.T) {
|
||||||
|
m := testfixtures.Load(t)
|
||||||
|
path := m.Case(t, testfixtures.CaseCoverDedup)[0]
|
||||||
|
|
||||||
|
p := newTestPlayer(t)
|
||||||
|
|
||||||
|
if got := p.format.SampleRate; got != speakerSampleRate {
|
||||||
|
t.Fatalf(
|
||||||
|
"precondition: a fresh player should hold the speaker "+
|
||||||
|
"rate, got %d",
|
||||||
|
got,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
if err := p.LoadFile(path); err != nil {
|
||||||
|
t.Fatalf("LoadFile(%s): %v", path, err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if p.format.SampleRate == speakerSampleRate {
|
||||||
|
t.Fatalf(
|
||||||
|
"p.format still holds the speaker rate (%d) after "+
|
||||||
|
"loading a %d Hz file: the replay path would "+
|
||||||
|
"resample from the wrong rate",
|
||||||
|
speakerSampleRate, fixtureSampleRate,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
if got := int(p.format.SampleRate); got != fixtureSampleRate {
|
||||||
|
t.Errorf(
|
||||||
|
"expected the decoder's rate %d, got %d",
|
||||||
|
fixtureSampleRate, got,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// trackLengthMs is written only when the database has a row for the
|
||||||
|
// file and cleared only by UnloadTrack, so a track with no row used
|
||||||
|
// to inherit whatever the last track's duration was -- and every
|
||||||
|
// position report is scaled by it, so the whole seek bar was then
|
||||||
|
// reporting one track's progress on another track's scale.
|
||||||
|
//
|
||||||
|
// There is no database here, so the lookup cannot succeed: exactly
|
||||||
|
// the case that used to inherit.
|
||||||
|
func TestLoadDoesNotInheritThePreviousTracksDuration(t *testing.T) {
|
||||||
|
m := testfixtures.Load(t)
|
||||||
|
path := m.Case(t, testfixtures.CaseCoverDedup)[0]
|
||||||
|
|
||||||
|
p := newTestPlayer(t)
|
||||||
|
|
||||||
|
// Stand in for a previous track whose duration was resolved.
|
||||||
|
p.trackLengthMs = 9_999_000
|
||||||
|
|
||||||
|
if err := p.LoadFile(path); err != nil {
|
||||||
|
t.Fatalf("LoadFile(%s): %v", path, err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if p.trackLengthMs == 9_999_000 {
|
||||||
|
t.Fatal(
|
||||||
|
"the previous track's duration survived the load: every " +
|
||||||
|
"position report for this track would be scaled by it",
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// A new chain supersedes the old one's pending finished callback.
|
||||||
|
// Without this, a callback that queued for p.mu behind a LoadFile
|
||||||
|
// woke up and rewound, stopped and auto-advanced the *new* track.
|
||||||
|
func TestANewChainSupersedesTheOldFinishedCallback(t *testing.T) {
|
||||||
|
m := testfixtures.Load(t)
|
||||||
|
paths := m.Case(t, testfixtures.CaseCoverDedup)
|
||||||
|
|
||||||
|
if len(paths) < 2 {
|
||||||
|
t.Skip("need two fixture tracks")
|
||||||
|
}
|
||||||
|
|
||||||
|
p := newTestPlayer(t)
|
||||||
|
|
||||||
|
if err := p.LoadFile(paths[0]); err != nil {
|
||||||
|
t.Fatalf("LoadFile(%s): %v", paths[0], err)
|
||||||
|
}
|
||||||
|
|
||||||
|
stale := p.chainID
|
||||||
|
|
||||||
|
if err := p.LoadFile(paths[1]); err != nil {
|
||||||
|
t.Fatalf("LoadFile(%s): %v", paths[1], err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if p.chainID == stale {
|
||||||
|
t.Fatal("loading a second file did not supersede the chain")
|
||||||
|
}
|
||||||
|
|
||||||
|
called := false
|
||||||
|
|
||||||
|
p.SetPlaybackFinishedHandler(func(error) { called = true })
|
||||||
|
|
||||||
|
// The first track's callback, arriving late.
|
||||||
|
p.onPlaybackFinished(stale, nil)
|
||||||
|
|
||||||
|
if called {
|
||||||
|
t.Error(
|
||||||
|
"a superseded chain's callback drove auto-advance: the " +
|
||||||
|
"track that is loaded now would be skipped",
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
if p.state == Stopped {
|
||||||
|
t.Error(
|
||||||
|
"a superseded chain's callback stopped the current track",
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// The decoder is read by the read-ahead goroutine and by every
|
||||||
|
// position emit, and those used to be guarded by different mutexes:
|
||||||
|
// the read by srcMu, the position by the speaker lock, which
|
||||||
|
// read-ahead never takes. Under -race this failed on the emit that
|
||||||
|
// LoadFile itself makes.
|
||||||
|
//
|
||||||
|
// It needs the read-ahead goroutine to actually be running, so it
|
||||||
|
// keeps asking for the position for long enough to overlap it.
|
||||||
|
func TestPositionReadsDoNotRaceTheReadAhead(t *testing.T) {
|
||||||
|
m := testfixtures.Load(t)
|
||||||
|
path := m.Case(t, testfixtures.CaseFLACAlbum)[0]
|
||||||
|
|
||||||
|
p := newTestPlayer(t)
|
||||||
|
|
||||||
|
if err := p.LoadFile(path); err != nil {
|
||||||
|
t.Fatalf("LoadFile(%s): %v", path, err)
|
||||||
|
}
|
||||||
|
|
||||||
|
for range 200 {
|
||||||
|
if _, err := p.CurrentPositionSeconds(); err != nil {
|
||||||
|
t.Fatalf("CurrentPositionSeconds: %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Seeking emits the landing position, and that emit reads the
|
||||||
|
// decoder -- so the source lock the seek holds must be released
|
||||||
|
// before it. A reentrant take here is a deadlock, not a failure,
|
||||||
|
// which is why this test exists rather than a comment.
|
||||||
|
func TestSeekEmitsWithoutDeadlocking(t *testing.T) {
|
||||||
|
m := testfixtures.Load(t)
|
||||||
|
path := m.Case(t, testfixtures.CaseFLACAlbum)[0]
|
||||||
|
|
||||||
|
p := newTestPlayer(t)
|
||||||
|
|
||||||
|
if err := p.LoadFile(path); err != nil {
|
||||||
|
t.Fatalf("LoadFile(%s): %v", path, err)
|
||||||
|
}
|
||||||
|
|
||||||
|
done := make(chan struct{})
|
||||||
|
|
||||||
|
go func() {
|
||||||
|
defer close(done)
|
||||||
|
|
||||||
|
_ = p.Seek(1)
|
||||||
|
}()
|
||||||
|
|
||||||
|
select {
|
||||||
|
case <-done:
|
||||||
|
case <-time.After(10 * time.Second):
|
||||||
|
t.Fatal("Seek deadlocked: the position emit re-took the source lock")
|
||||||
|
}
|
||||||
|
}
|
||||||
+171
-34
@@ -52,9 +52,17 @@ type Player struct {
|
|||||||
control *beep.Ctrl
|
control *beep.Ctrl
|
||||||
volume *effects.Volume
|
volume *effects.Volume
|
||||||
speakerStreamer beep.Streamer
|
speakerStreamer beep.Streamer
|
||||||
playbackFinishedHandler func()
|
playbackFinishedHandler func(error)
|
||||||
trackChangeID uint64
|
trackChangeID uint64
|
||||||
mediaControls mediacontrols.Handler
|
|
||||||
|
// chainID identifies the streamer chain currently registered with
|
||||||
|
// the speaker. updateStreamers bumps it, and the finished
|
||||||
|
// callback carries the value it was registered with, so a callback
|
||||||
|
// that queued for p.mu behind a LoadFile can tell that the player
|
||||||
|
// has moved on and return rather than rewinding somebody else's
|
||||||
|
// track.
|
||||||
|
chainID uint64
|
||||||
|
mediaControls mediacontrols.Handler
|
||||||
|
|
||||||
// duckAmount is the attenuation currently applied on top of the
|
// duckAmount is the attenuation currently applied on top of the
|
||||||
// user's volume, in the same base-2 exponent effects.Volume uses.
|
// user's volume, in the same base-2 exponent effects.Volume uses.
|
||||||
@@ -180,11 +188,18 @@ func (p *Player) InitSpeaker() error {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// SetPlaybackFinishedHandler sets a callback invoked when a track
|
// SetPlaybackFinishedHandler sets a callback invoked when a track
|
||||||
// finishes naturally. This allows the queue to drive auto-advance
|
// stops streaming. This allows the queue to drive auto-advance
|
||||||
// without circular imports.
|
// without circular imports.
|
||||||
//
|
//
|
||||||
|
// The error says *why* the track stopped: nil for a track that
|
||||||
|
// reached its end, non-nil for one that broke partway through. Both
|
||||||
|
// arrive here because both look identical to the speaker, and only
|
||||||
|
// the queue holds the metadata a PlaybackFailed needs -- but they are
|
||||||
|
// not the same event, and reporting a decode failure as a natural
|
||||||
|
// finish is how a broken file used to auto-advance in silence.
|
||||||
|
//
|
||||||
//wails:ignore // internal wiring, not part of the app's IPC surface.
|
//wails:ignore // internal wiring, not part of the app's IPC surface.
|
||||||
func (p *Player) SetPlaybackFinishedHandler(handler func()) {
|
func (p *Player) SetPlaybackFinishedHandler(handler func(error)) {
|
||||||
p.mu.Lock()
|
p.mu.Lock()
|
||||||
defer p.mu.Unlock()
|
defer p.mu.Unlock()
|
||||||
|
|
||||||
@@ -424,6 +439,19 @@ func (p *Player) updateStreamers(
|
|||||||
newBaseStreamer beep.StreamSeeker,
|
newBaseStreamer beep.StreamSeeker,
|
||||||
sr beep.SampleRate,
|
sr beep.SampleRate,
|
||||||
) error {
|
) error {
|
||||||
|
// A new chain supersedes the old one, so any finished callback the
|
||||||
|
// old one still owes is stale from here on.
|
||||||
|
p.chainID++
|
||||||
|
|
||||||
|
// The previous read-ahead goroutine reads the same decoder this
|
||||||
|
// one is about to, under its own srcMu -- two goroutines, two
|
||||||
|
// mutexes, one decoder that is not safe for concurrent use. The
|
||||||
|
// replay-after-finish path rebuilds from p.seeker without going
|
||||||
|
// through LoadFile, which is where that pair could meet.
|
||||||
|
if p.buffered != nil {
|
||||||
|
p.buffered.Close()
|
||||||
|
}
|
||||||
|
|
||||||
// set base streamer
|
// set base streamer
|
||||||
p.baseStreamer = newBaseStreamer
|
p.baseStreamer = newBaseStreamer
|
||||||
p.seeker = newBaseStreamer
|
p.seeker = newBaseStreamer
|
||||||
@@ -474,23 +502,57 @@ func (p *Player) startPaused() {
|
|||||||
p.control.Paused = true
|
p.control.Paused = true
|
||||||
speaker.Unlock()
|
speaker.Unlock()
|
||||||
|
|
||||||
|
// Captured, not read at callback time: by then p.chainID names
|
||||||
|
// whatever is loaded *now*, which is the thing the guard exists to
|
||||||
|
// distinguish this chain from.
|
||||||
|
chainID := p.chainID
|
||||||
|
buffered := p.buffered
|
||||||
|
|
||||||
// The beep.Callback runs with the speaker mutex held, so we
|
// The beep.Callback runs with the speaker mutex held, so we
|
||||||
// dispatch to a goroutine that can safely acquire p.mu.
|
// dispatch to a goroutine that can safely acquire p.mu.
|
||||||
speaker.Play(beep.Seq(
|
speaker.Play(beep.Seq(
|
||||||
p.speakerStreamer,
|
p.speakerStreamer,
|
||||||
beep.Callback(func() {
|
beep.Callback(func() {
|
||||||
go p.onPlaybackFinished()
|
// Asked here rather than under p.mu: this is the chain that
|
||||||
|
// just ended, and by the time the goroutine holds the lock
|
||||||
|
// p.buffered may be a different one.
|
||||||
|
var err error
|
||||||
|
if buffered != nil {
|
||||||
|
err = buffered.Err()
|
||||||
|
}
|
||||||
|
|
||||||
|
go p.onPlaybackFinished(chainID, err)
|
||||||
}),
|
}),
|
||||||
))
|
))
|
||||||
|
|
||||||
p.state = Paused
|
p.state = Paused
|
||||||
}
|
}
|
||||||
|
|
||||||
// onPlaybackFinished handles the natural end of a track. It is
|
// onPlaybackFinished handles a track that stopped streaming, whether
|
||||||
// called on a new goroutine from the beep callback (which holds
|
// it ended or broke. It is called on a new goroutine from the beep
|
||||||
// the speaker lock) so that it can safely acquire p.mu.
|
// callback (which holds the speaker lock) so that it can safely
|
||||||
func (p *Player) onPlaybackFinished() {
|
// acquire p.mu.
|
||||||
|
//
|
||||||
|
// chainID names the streamer chain the callback fired for and srcErr
|
||||||
|
// says why it stopped.
|
||||||
|
func (p *Player) onPlaybackFinished(chainID uint64, srcErr error) {
|
||||||
p.mu.Lock()
|
p.mu.Lock()
|
||||||
|
|
||||||
|
// The player has moved on while this callback queued for the lock
|
||||||
|
// -- a user pressing Next during the last second of a track is
|
||||||
|
// enough. Everything below is about the *current* track: rewinding
|
||||||
|
// the decoder, saying playback stopped, asking the queue to
|
||||||
|
// advance. Doing any of it now would do it to the wrong track.
|
||||||
|
if chainID != p.chainID {
|
||||||
|
p.mu.Unlock()
|
||||||
|
p.logger.Debug(
|
||||||
|
"Ignoring finished callback for a superseded chain",
|
||||||
|
"chain", chainID, "current", p.chainID,
|
||||||
|
)
|
||||||
|
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
p.state = Stopped
|
p.state = Stopped
|
||||||
handler := p.playbackFinishedHandler
|
handler := p.playbackFinishedHandler
|
||||||
mc := p.mediaControls
|
mc := p.mediaControls
|
||||||
@@ -501,10 +563,11 @@ func (p *Player) onPlaybackFinished() {
|
|||||||
// the Stopped state anyway, so this only moves the decoder.
|
// the Stopped state anyway, so this only moves the decoder.
|
||||||
p.rewindLocked()
|
p.rewindLocked()
|
||||||
p.emitPositionLocked()
|
p.emitPositionLocked()
|
||||||
p.mu.Unlock()
|
|
||||||
|
|
||||||
// Emit Wails events outside the lock — these are non-blocking
|
// Emitted under p.mu, like every other transition in this file.
|
||||||
// calls that don't need player state.
|
// Outside it, a Play() taking the lock in the gap emits `playing`
|
||||||
|
// first and this stale `stopped` lands last -- leaving the button
|
||||||
|
// showing play over a track that is audibly running.
|
||||||
p.emitPlaybackFinished()
|
p.emitPlaybackFinished()
|
||||||
|
|
||||||
events.Emit(
|
events.Emit(
|
||||||
@@ -513,6 +576,8 @@ func (p *Player) onPlaybackFinished() {
|
|||||||
map[string]string{"state": string(Stopped)},
|
map[string]string{"state": string(Stopped)},
|
||||||
)
|
)
|
||||||
|
|
||||||
|
p.mu.Unlock()
|
||||||
|
|
||||||
// Notify media controls outside the lock. The track just
|
// Notify media controls outside the lock. The track just
|
||||||
// ended so position is 0.
|
// ended so position is 0.
|
||||||
if mc != nil {
|
if mc != nil {
|
||||||
@@ -521,12 +586,19 @@ func (p *Player) onPlaybackFinished() {
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
p.logger.Info("Playback finished naturally")
|
if srcErr != nil {
|
||||||
|
p.logger.Error(
|
||||||
|
"Playback stopped: the audio source failed",
|
||||||
|
"err", srcErr,
|
||||||
|
)
|
||||||
|
} else {
|
||||||
|
p.logger.Info("Playback finished naturally")
|
||||||
|
}
|
||||||
|
|
||||||
// Notify queue for auto-advance. Called without p.mu held
|
// Notify queue for auto-advance. Called without p.mu held
|
||||||
// because it re-enters the player via LoadFile/Play.
|
// because it re-enters the player via LoadFile/Play.
|
||||||
if handler != nil {
|
if handler != nil {
|
||||||
handler()
|
handler(srcErr)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -587,6 +659,18 @@ func (p *Player) loadFileLocked(filePath string) error {
|
|||||||
|
|
||||||
p.currentFile = f
|
p.currentFile = f
|
||||||
|
|
||||||
|
// The decoder's own format, kept for the paths that rebuild the
|
||||||
|
// chain later: Play()'s replay branch resamples from it, so a
|
||||||
|
// stale rate there plays a finished track back at the wrong speed.
|
||||||
|
p.format = format
|
||||||
|
|
||||||
|
// The previous track's duration must not outlive it. This is set
|
||||||
|
// again by emitTrackChanged below, but only when the database has
|
||||||
|
// a row for the file -- and every position this player reports is
|
||||||
|
// scaled by it, so inheriting means every report is wrong by the
|
||||||
|
// ratio between two unrelated tracks.
|
||||||
|
p.trackLengthMs = 0
|
||||||
|
|
||||||
if err := p.updateStreamers(
|
if err := p.updateStreamers(
|
||||||
streamer, format.SampleRate,
|
streamer, format.SampleRate,
|
||||||
); err != nil {
|
); err != nil {
|
||||||
@@ -906,6 +990,8 @@ func (p *Player) CurrentPosition() (int, error) {
|
|||||||
return 0, errNoAudioFileLoaded
|
return 0, errNoAudioFileLoaded
|
||||||
}
|
}
|
||||||
|
|
||||||
|
defer p.lockSourceLocked()()
|
||||||
|
|
||||||
speaker.Lock()
|
speaker.Lock()
|
||||||
pos := math.Round(
|
pos := math.Round(
|
||||||
100.0 * float64(p.seeker.Position()) /
|
100.0 * float64(p.seeker.Position()) /
|
||||||
@@ -924,6 +1010,28 @@ func (p *Player) Seek(targetSeconds int) error {
|
|||||||
return p.seekLocked(targetSeconds)
|
return p.seekLocked(targetSeconds)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// lockSourceLocked blocks the read-ahead goroutine from touching the
|
||||||
|
// decoder and returns the function that releases it, so a caller can
|
||||||
|
// `defer p.lockSourceLocked()()`.
|
||||||
|
//
|
||||||
|
// Reading the decoder's position is a read *of the decoder*, and the
|
||||||
|
// speaker lock does not exclude the read-ahead goroutine -- it never
|
||||||
|
// takes it. That was a genuine data race on every position emit,
|
||||||
|
// once a second for the whole of playback.
|
||||||
|
//
|
||||||
|
// srcMu is not reentrant, so nothing that already holds it may call
|
||||||
|
// this; seekSourceLocked exists to keep that region free of emits.
|
||||||
|
// Must be called with p.mu held.
|
||||||
|
func (p *Player) lockSourceLocked() func() {
|
||||||
|
if p.buffered == nil {
|
||||||
|
return func() {}
|
||||||
|
}
|
||||||
|
|
||||||
|
p.buffered.LockSource()
|
||||||
|
|
||||||
|
return p.buffered.UnlockSource
|
||||||
|
}
|
||||||
|
|
||||||
// rewindLocked returns the decoder to the start of the track without
|
// rewindLocked returns the decoder to the start of the track without
|
||||||
// touching playback state. Must be called with p.mu held.
|
// touching playback state. Must be called with p.mu held.
|
||||||
func (p *Player) rewindLocked() {
|
func (p *Player) rewindLocked() {
|
||||||
@@ -959,6 +1067,46 @@ func (p *Player) seekLocked(targetSeconds int) error {
|
|||||||
return fmt.Errorf("cannot get track length: %w", err)
|
return fmt.Errorf("cannot get track length: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// The source lock is released before anything below is emitted:
|
||||||
|
// emitPositionLocked reads the decoder's position and takes the
|
||||||
|
// same lock, which is not reentrant.
|
||||||
|
seekErr := p.seekSourceLocked(targetSeconds, lengthSecs)
|
||||||
|
if seekErr != nil {
|
||||||
|
p.logger.Warn(
|
||||||
|
"Seek failed, playback will start from "+
|
||||||
|
"the beginning",
|
||||||
|
"target-seconds", targetSeconds,
|
||||||
|
"err", seekErr,
|
||||||
|
)
|
||||||
|
|
||||||
|
// The optimistic move the UI already made has to be taken
|
||||||
|
// back, and only the backend knows it did not happen.
|
||||||
|
events.Emit(p.ctx, events.SeekFailed)
|
||||||
|
p.emitPositionLocked()
|
||||||
|
|
||||||
|
return fmt.Errorf("failed to seek: %w", seekErr)
|
||||||
|
}
|
||||||
|
|
||||||
|
if p.mediaControls != nil {
|
||||||
|
p.mediaControls.NotifySeek(targetSeconds)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Report the landing position immediately rather than leaving the
|
||||||
|
// UI to guess until the next tick — this is the half of H-3 that
|
||||||
|
// desynced the seek bar by 30 s over four keyboard seeks.
|
||||||
|
p.emitPositionLocked()
|
||||||
|
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// seekSourceLocked moves the decoder and flushes the stale read-ahead
|
||||||
|
// behind it. It owns the source lock for exactly that long and
|
||||||
|
// emits nothing, so its caller is free to read the position
|
||||||
|
// afterwards. Must be called with p.mu held.
|
||||||
|
func (p *Player) seekSourceLocked(
|
||||||
|
targetSeconds int,
|
||||||
|
lengthSecs int,
|
||||||
|
) error {
|
||||||
// Block the read-ahead goroutine from reading the source while
|
// Block the read-ahead goroutine from reading the source while
|
||||||
// we seek it. The decoder (e.g. FLAC's bufseekio.ReadSeeker) is
|
// we seek it. The decoder (e.g. FLAC's bufseekio.ReadSeeker) is
|
||||||
// not safe for concurrent Read+Seek, and read-ahead runs on its
|
// not safe for concurrent Read+Seek, and read-ahead runs on its
|
||||||
@@ -1014,19 +1162,11 @@ func (p *Player) seekLocked(targetSeconds int) error {
|
|||||||
if seekErr != nil {
|
if seekErr != nil {
|
||||||
speaker.Unlock()
|
speaker.Unlock()
|
||||||
|
|
||||||
p.logger.Warn(
|
p.logger.Debug(
|
||||||
"Seek failed, playback will start from "+
|
"seek rejected by the decoder",
|
||||||
"the beginning",
|
"samples", samples, "err", seekErr,
|
||||||
"target-seconds", targetSeconds,
|
|
||||||
"samples", samples,
|
|
||||||
"err", seekErr,
|
|
||||||
)
|
)
|
||||||
|
|
||||||
// The optimistic move the UI already made has to be taken
|
|
||||||
// back, and only the backend knows it did not happen.
|
|
||||||
events.Emit(p.ctx, events.SeekFailed)
|
|
||||||
p.emitPositionLocked()
|
|
||||||
|
|
||||||
return fmt.Errorf("failed to seek: %w", seekErr)
|
return fmt.Errorf("failed to seek: %w", seekErr)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1039,15 +1179,6 @@ func (p *Player) seekLocked(targetSeconds int) error {
|
|||||||
p.buffered.Flush()
|
p.buffered.Flush()
|
||||||
}
|
}
|
||||||
|
|
||||||
if p.mediaControls != nil {
|
|
||||||
p.mediaControls.NotifySeek(targetSeconds)
|
|
||||||
}
|
|
||||||
|
|
||||||
// Report the landing position immediately rather than leaving the
|
|
||||||
// UI to guess until the next tick — this is the half of H-3 that
|
|
||||||
// desynced the seek bar by 30 s over four keyboard seeks.
|
|
||||||
p.emitPositionLocked()
|
|
||||||
|
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1140,6 +1271,10 @@ func (p *Player) seekerLengthSecsLocked() (int, error) {
|
|||||||
return 0, errNoAudioFileLoaded
|
return 0, errNoAudioFileLoaded
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Len is fixed for the life of the decoder, so unlike Position it
|
||||||
|
// races with nothing and needs no source lock -- which it must not
|
||||||
|
// take anyway: displayPositionSecsLocked calls this while holding
|
||||||
|
// it, and srcMu is not reentrant.
|
||||||
speaker.Lock()
|
speaker.Lock()
|
||||||
length := p.seeker.Len() / int(p.format.SampleRate)
|
length := p.seeker.Len() / int(p.format.SampleRate)
|
||||||
speaker.Unlock()
|
speaker.Unlock()
|
||||||
@@ -1156,6 +1291,8 @@ func (p *Player) displayPositionSecsLocked() int {
|
|||||||
return 0
|
return 0
|
||||||
}
|
}
|
||||||
|
|
||||||
|
defer p.lockSourceLocked()()
|
||||||
|
|
||||||
speaker.Lock()
|
speaker.Lock()
|
||||||
pos := p.seeker.Position()
|
pos := p.seeker.Position()
|
||||||
total := p.seeker.Len()
|
total := p.seeker.Len()
|
||||||
|
|||||||
@@ -83,7 +83,7 @@ func TestFallback_TriggersOnNaturalFinish(t *testing.T) {
|
|||||||
q.SetFallbackSource(fake)
|
q.SetFallbackSource(fake)
|
||||||
|
|
||||||
q.SetQueue(seedPaths, 0, false, Source{Type: "album", ID: 1, Label: "Seed Album"})
|
q.SetQueue(seedPaths, 0, false, Source{Type: "album", ID: 1, Label: "Seed Album"})
|
||||||
q.OnPlaybackFinished()
|
q.OnPlaybackFinished(nil)
|
||||||
|
|
||||||
waitUntil(t, func() bool { return fake.callCount() == 1 }, "fallback to be resolved")
|
waitUntil(t, func() bool { return fake.callCount() == 1 }, "fallback to be resolved")
|
||||||
waitUntil(t, func() bool {
|
waitUntil(t, func() bool {
|
||||||
@@ -159,7 +159,7 @@ func TestFallback_EmptyResultLeavesQueueExhausted(t *testing.T) {
|
|||||||
q.SetFallbackSource(fake)
|
q.SetFallbackSource(fake)
|
||||||
|
|
||||||
q.SetQueue(seedPaths, 0, false, Source{})
|
q.SetQueue(seedPaths, 0, false, Source{})
|
||||||
q.OnPlaybackFinished()
|
q.OnPlaybackFinished(nil)
|
||||||
|
|
||||||
waitUntil(t, func() bool { return fake.callCount() == 1 }, "fallback to be resolved")
|
waitUntil(t, func() bool { return fake.callCount() == 1 }, "fallback to be resolved")
|
||||||
|
|
||||||
@@ -193,7 +193,7 @@ func TestFallback_StaleResolutionDiscarded(t *testing.T) {
|
|||||||
q.SetFallbackSource(fake)
|
q.SetFallbackSource(fake)
|
||||||
|
|
||||||
q.SetQueue(seedPaths, 0, false, Source{})
|
q.SetQueue(seedPaths, 0, false, Source{})
|
||||||
q.OnPlaybackFinished() // starts resolving, blocked on gate
|
q.OnPlaybackFinished(nil) // starts resolving, blocked on gate
|
||||||
|
|
||||||
time.Sleep(20 * time.Millisecond) // let the goroutine reach the gate
|
time.Sleep(20 * time.Millisecond) // let the goroutine reach the gate
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,78 @@
|
|||||||
|
package queue
|
||||||
|
|
||||||
|
import (
|
||||||
|
"errors"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"yellowjacket/backend/events"
|
||||||
|
)
|
||||||
|
|
||||||
|
// errTestDecode stands in for a decoder blowing up mid-track.
|
||||||
|
var errTestDecode = errors.New("decode blew up")
|
||||||
|
|
||||||
|
// currentIndex == -1 against a non-empty queue is a state this
|
||||||
|
// package produces on purpose: onQueueExhausted(false) sets it and
|
||||||
|
// deliberately leaves the finished track loaded in the player, so it
|
||||||
|
// stays on the now-playing bar. Pressing play from there and letting
|
||||||
|
// it finish re-enters OnPlaybackFinished with exactly that pair --
|
||||||
|
// which used to index q.tracks[-1] and panic, on a goroutine
|
||||||
|
// dispatched from the audio callback with no caller to recover it.
|
||||||
|
func TestFinishedWithNoCurrentTrackDoesNotPanic(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
index int
|
||||||
|
}{
|
||||||
|
{"exhausted queue leaves -1", -1},
|
||||||
|
{"index past the end", 3},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
q, _, _ := setupRecordedQueue(t)
|
||||||
|
q.tracks = []Track{
|
||||||
|
{FilePath: "/a.mp3"},
|
||||||
|
{FilePath: "/b.mp3"},
|
||||||
|
}
|
||||||
|
q.currentIndex = tt.index
|
||||||
|
|
||||||
|
// The assertion is that this returns at all.
|
||||||
|
q.OnPlaybackFinished(nil)
|
||||||
|
|
||||||
|
if q.currentIndex != tt.index {
|
||||||
|
t.Errorf(
|
||||||
|
"an out-of-range index was acted on: %d became %d",
|
||||||
|
tt.index, q.currentIndex,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// A track that broke mid-playback is not a track that was listened
|
||||||
|
// to. The player cannot say so itself -- the metadata is here -- so
|
||||||
|
// it hands the reason over and this is where it becomes a
|
||||||
|
// PlaybackFailed rather than a silent auto-advance.
|
||||||
|
func TestAFailedTrackIsReportedAndNotCountedAsAPlay(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
q, _, rec := setupRecordedQueue(t)
|
||||||
|
q.tracks = []Track{
|
||||||
|
{FilePath: "/a.mp3", Title: "A", AudioFileID: 1},
|
||||||
|
{FilePath: "/b.mp3", Title: "B", AudioFileID: 2},
|
||||||
|
}
|
||||||
|
q.currentIndex = 0
|
||||||
|
|
||||||
|
q.OnPlaybackFinished(errTestDecode)
|
||||||
|
|
||||||
|
if _, ok := rec.Last(events.PlaybackFailed); !ok {
|
||||||
|
t.Errorf(
|
||||||
|
"a track that failed mid-playback told the user nothing; "+
|
||||||
|
"got %v",
|
||||||
|
rec.Names(),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -1,18 +1,45 @@
|
|||||||
package queue
|
package queue
|
||||||
|
|
||||||
// OnPlaybackFinished is called when a track finishes playing naturally.
|
// OnPlaybackFinished is called when a track stops streaming. This
|
||||||
// This drives the auto-advance behavior and records the play.
|
// drives the auto-advance behavior and records the play.
|
||||||
func (q *Queue) OnPlaybackFinished() {
|
//
|
||||||
|
// srcErr says why the track stopped: nil for one that reached its
|
||||||
|
// end, non-nil for one that broke partway through. The player cannot
|
||||||
|
// tell the user which, because the metadata lives here -- so a failure
|
||||||
|
// is reported as PlaybackFailed and *not* recorded as a play, while
|
||||||
|
// the advance happens either way. Before this, a file that failed
|
||||||
|
// mid-track advanced in silence and was counted as listened to.
|
||||||
|
//
|
||||||
|
//wails:ignore // internal wiring, not part of the app's IPC surface.
|
||||||
|
func (q *Queue) OnPlaybackFinished(srcErr error) {
|
||||||
q.mu.Lock()
|
q.mu.Lock()
|
||||||
|
|
||||||
if len(q.tracks) == 0 {
|
// currentIndex is -1 whenever the queue has been exhausted, and
|
||||||
|
// onQueueExhausted deliberately leaves the finished track loaded
|
||||||
|
// in the player -- so a natural finish can re-enter here against a
|
||||||
|
// queue that is not empty and an index that is not valid. Every
|
||||||
|
// other path in this package bounds-checks before indexing; this
|
||||||
|
// one panicked, on a goroutine with no caller to recover it.
|
||||||
|
if q.currentIndex < 0 || q.currentIndex >= len(q.tracks) {
|
||||||
q.mu.Unlock()
|
q.mu.Unlock()
|
||||||
|
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
// Capture the track that just finished before advancing.
|
// Capture the track that just finished before advancing.
|
||||||
finishedID := q.tracks[q.currentIndex].AudioFileID
|
finished := q.tracks[q.currentIndex]
|
||||||
|
finishedID := finished.AudioFileID
|
||||||
|
|
||||||
|
if srcErr != nil {
|
||||||
|
q.emitPlaybackFailed(finished, srcErr)
|
||||||
|
}
|
||||||
|
|
||||||
|
// A track that broke was not listened to.
|
||||||
|
recordFinished := func() {
|
||||||
|
if srcErr == nil {
|
||||||
|
q.recordPlay(finishedID)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Repeat One: replay the current track.
|
// Repeat One: replay the current track.
|
||||||
if q.repeatMode == RepeatOne {
|
if q.repeatMode == RepeatOne {
|
||||||
@@ -21,7 +48,7 @@ func (q *Queue) OnPlaybackFinished() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
q.mu.Unlock()
|
q.mu.Unlock()
|
||||||
q.recordPlay(finishedID)
|
recordFinished()
|
||||||
|
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
@@ -31,7 +58,7 @@ func (q *Queue) OnPlaybackFinished() {
|
|||||||
// Queue exhausted — this is the extension point for a future fallback playlist.
|
// Queue exhausted — this is the extension point for a future fallback playlist.
|
||||||
q.onQueueExhausted(false)
|
q.onQueueExhausted(false)
|
||||||
q.mu.Unlock()
|
q.mu.Unlock()
|
||||||
q.recordPlay(finishedID)
|
recordFinished()
|
||||||
|
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
@@ -44,12 +71,12 @@ func (q *Queue) OnPlaybackFinished() {
|
|||||||
if !q.playCurrentOrSkip(true, q.nextIndex) {
|
if !q.playCurrentOrSkip(true, q.nextIndex) {
|
||||||
q.onQueueExhausted(false)
|
q.onQueueExhausted(false)
|
||||||
q.mu.Unlock()
|
q.mu.Unlock()
|
||||||
q.recordPlay(finishedID)
|
recordFinished()
|
||||||
|
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
q.emitIndexChanged()
|
q.emitIndexChanged()
|
||||||
q.mu.Unlock()
|
q.mu.Unlock()
|
||||||
q.recordPlay(finishedID)
|
recordFinished()
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -107,7 +107,7 @@ func TestPlaybackFailed_AutoAdvanceSkipsPastIt(t *testing.T) {
|
|||||||
|
|
||||||
// The first track finished: auto-advance lands on the missing
|
// The first track finished: auto-advance lands on the missing
|
||||||
// file and must step over it rather than stopping dead.
|
// file and must step over it rather than stopping dead.
|
||||||
q.OnPlaybackFinished()
|
q.OnPlaybackFinished(nil)
|
||||||
|
|
||||||
if got := q.GetState().CurrentIndex; got != 2 {
|
if got := q.GetState().CurrentIndex; got != 2 {
|
||||||
t.Errorf("currentIndex after skipping: got %d, want 2", got)
|
t.Errorf("currentIndex after skipping: got %d, want 2", got)
|
||||||
@@ -183,7 +183,7 @@ func TestQueueExhausted_KeepsTheFinishedTrackLoaded(t *testing.T) {
|
|||||||
|
|
||||||
q.SetQueue(paths, 0, false, Source{})
|
q.SetQueue(paths, 0, false, Source{})
|
||||||
q.Play()
|
q.Play()
|
||||||
q.OnPlaybackFinished()
|
q.OnPlaybackFinished(nil)
|
||||||
|
|
||||||
if q.GetState().CurrentIndex != -1 {
|
if q.GetState().CurrentIndex != -1 {
|
||||||
t.Errorf(
|
t.Errorf(
|
||||||
|
|||||||
@@ -1,6 +1,12 @@
|
|||||||
import { test, expect } from '../support/fixtures.js';
|
import { test, expect } from '../support/fixtures.js';
|
||||||
import type { Page } from '@playwright/test';
|
import type { Page } from '@playwright/test';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* How far the scroll test scrolls. One constant, because the guard and
|
||||||
|
* the assertion have to agree about it — they did not, which is #133.
|
||||||
|
*/
|
||||||
|
const SCROLL_TARGET = 80;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Plan 007 phase 5: expanding an album shows its tracks.
|
* Plan 007 phase 5: expanding an album shows its tracks.
|
||||||
*
|
*
|
||||||
@@ -104,20 +110,27 @@ test.describe('the album dropdown', () => {
|
|||||||
await app.setViewportSize({ width: 900, height: 600 });
|
await app.setViewportSize({ width: 900, height: 600 });
|
||||||
|
|
||||||
try {
|
try {
|
||||||
await expect.poll(() => scrollRange(app)).toMatchObject({
|
// Wait for the range the assertion below actually needs, not for
|
||||||
scrollable: true,
|
// "scrollable at all" (#133). The guard used to be
|
||||||
overflowY: 'auto',
|
// `scrollHeight > clientHeight + 40` while the next line asks to
|
||||||
});
|
// reach 80, so any range in 41-79 satisfied it and could not
|
||||||
|
// satisfy the assertion — and the grid passes through exactly
|
||||||
|
// that while it settles, because it recomputes its columns after
|
||||||
|
// the resize rather than during it. The settled range here is
|
||||||
|
// 330, so this waits rather than weakening anything.
|
||||||
|
await expect
|
||||||
|
.poll(() => scrollRange(app))
|
||||||
|
.toMatchObject({ room: true, overflowY: 'auto' });
|
||||||
|
|
||||||
await app.evaluate(() => {
|
await app.evaluate((target) => {
|
||||||
const sc = document
|
const sc = document
|
||||||
.querySelector('cover-grid')
|
.querySelector('cover-grid')
|
||||||
?.shadowRoot?.querySelector('.grid-scroll-container');
|
?.shadowRoot?.querySelector('.grid-scroll-container');
|
||||||
|
|
||||||
if (sc) sc.scrollTop = 80;
|
if (sc) sc.scrollTop = target;
|
||||||
});
|
}, SCROLL_TARGET);
|
||||||
|
|
||||||
expect(await scrollTop(app)).toBe(80);
|
expect(await scrollTop(app)).toBe(SCROLL_TARGET);
|
||||||
|
|
||||||
// And the dropdown it opens is on screen, wherever the manager
|
// And the dropdown it opens is on screen, wherever the manager
|
||||||
// decides that leaves the scroll. It is *not* "the position is
|
// decides that leaves the scroll. It is *not* "the position is
|
||||||
@@ -250,16 +263,19 @@ async function closeDropdown(app: Page): Promise<void> {
|
|||||||
|
|
||||||
/** Whether the grid can scroll at all, which decides if a probe can move. */
|
/** Whether the grid can scroll at all, which decides if a probe can move. */
|
||||||
async function scrollRange(app: Page) {
|
async function scrollRange(app: Page) {
|
||||||
return app.evaluate(() => {
|
return app.evaluate((target) => {
|
||||||
const sc = document
|
const sc = document
|
||||||
.querySelector('cover-grid')
|
.querySelector('cover-grid')
|
||||||
?.shadowRoot?.querySelector('.grid-scroll-container');
|
?.shadowRoot?.querySelector('.grid-scroll-container');
|
||||||
|
|
||||||
return {
|
return {
|
||||||
scrollable: !!sc && sc.scrollHeight > sc.clientHeight + 40,
|
// `room` is the precondition of the assertion that follows it:
|
||||||
|
// enough range to actually reach the target. A threshold below
|
||||||
|
// what the caller depends on is not a guard.
|
||||||
|
room: !!sc && sc.scrollHeight - sc.clientHeight >= target,
|
||||||
overflowY: sc ? getComputedStyle(sc).overflowY : '',
|
overflowY: sc ? getComputedStyle(sc).overflowY : '',
|
||||||
};
|
};
|
||||||
});
|
}, SCROLL_TARGET);
|
||||||
}
|
}
|
||||||
|
|
||||||
async function scrollTop(app: Page): Promise<number> {
|
async function scrollTop(app: Page): Promise<number> {
|
||||||
|
|||||||
@@ -113,14 +113,6 @@ export function Next(): $CancellablePromise<void> {
|
|||||||
return $Call.ByID(1968784044);
|
return $Call.ByID(1968784044);
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
|
||||||
* OnPlaybackFinished is called when a track finishes playing naturally.
|
|
||||||
* This drives the auto-advance behavior and records the play.
|
|
||||||
*/
|
|
||||||
export function OnPlaybackFinished(): $CancellablePromise<void> {
|
|
||||||
return $Call.ByID(2184869763);
|
|
||||||
}
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Play handles a play request by either resuming the current track or
|
* Play handles a play request by either resuming the current track or
|
||||||
* starting playback from the beginning of the queue. When a track is
|
* starting playback from the beginning of the queue. When a track is
|
||||||
|
|||||||
@@ -160,29 +160,52 @@ export class HomeView extends ViewLifecycleMixin(LitElement) {
|
|||||||
user-select: none;
|
user-select: none;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/*
|
||||||
|
* The hover play button is a *hover* affordance, so it is
|
||||||
|
* gated on the device having hover rather than on width. A
|
||||||
|
* touch long-press synthesises a hover state in the WebView,
|
||||||
|
* so on a phone it flashed into view during the 500ms hold
|
||||||
|
* that utils/long-press.ts is measuring for a context menu —
|
||||||
|
* a control appearing because you were reaching for a
|
||||||
|
* different one. A phone user taps the album and plays from
|
||||||
|
* the detail view, so there is nothing to replace it with.
|
||||||
|
*
|
||||||
|
* display:none outside the query rather than opacity:0 on
|
||||||
|
* its own: an opacity-0 button still takes taps and is
|
||||||
|
* still in the accessibility tree, so the invisible control
|
||||||
|
* would keep the hit area it was never meant to have on
|
||||||
|
* touch. Everything else stays inside, so the desktop
|
||||||
|
* animation is unchanged.
|
||||||
|
*/
|
||||||
.play {
|
.play {
|
||||||
position: absolute;
|
display: none;
|
||||||
right: 8px;
|
|
||||||
bottom: 8px;
|
|
||||||
width: 38px;
|
|
||||||
height: 38px;
|
|
||||||
border: none;
|
|
||||||
border-radius: 50%;
|
|
||||||
background: var(--yj-accent, #ffd43b);
|
|
||||||
color: var(--yj-accent-fg, #000);
|
|
||||||
display: flex;
|
|
||||||
align-items: center;
|
|
||||||
justify-content: center;
|
|
||||||
cursor: pointer;
|
|
||||||
opacity: 0;
|
|
||||||
transform: translateY(6px);
|
|
||||||
transition: opacity 0.12s ease, transform 0.12s ease;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
.card:hover .play,
|
@media (hover: hover) and (pointer: fine) {
|
||||||
.card:focus-within .play {
|
.play {
|
||||||
opacity: 1;
|
position: absolute;
|
||||||
transform: translateY(0);
|
right: 8px;
|
||||||
|
bottom: 8px;
|
||||||
|
width: 38px;
|
||||||
|
height: 38px;
|
||||||
|
border: none;
|
||||||
|
border-radius: 50%;
|
||||||
|
background: var(--yj-accent, #ffd43b);
|
||||||
|
color: var(--yj-accent-fg, #000);
|
||||||
|
display: flex;
|
||||||
|
align-items: center;
|
||||||
|
justify-content: center;
|
||||||
|
cursor: pointer;
|
||||||
|
opacity: 0;
|
||||||
|
transform: translateY(6px);
|
||||||
|
transition: opacity 0.12s ease, transform 0.12s ease;
|
||||||
|
}
|
||||||
|
|
||||||
|
.card:hover .play,
|
||||||
|
.card:focus-within .play {
|
||||||
|
opacity: 1;
|
||||||
|
transform: translateY(0);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
.name {
|
.name {
|
||||||
|
|||||||
@@ -13,6 +13,7 @@ import {
|
|||||||
isQueueSourceNavigable,
|
isQueueSourceNavigable,
|
||||||
navigateToQueueSource,
|
navigateToQueueSource,
|
||||||
} from '@utils/queue-source-link';
|
} from '@utils/queue-source-link';
|
||||||
|
import { PHONE_QUERY } from '@utils/breakpoints';
|
||||||
import { PlayerController } from '@store/controllers/player-controller';
|
import { PlayerController } from '@store/controllers/player-controller';
|
||||||
import { creditStore } from '@store/credit-store';
|
import { creditStore } from '@store/credit-store';
|
||||||
import { QueueController } from '@store/controllers/queue-controller';
|
import { QueueController } from '@store/controllers/queue-controller';
|
||||||
@@ -80,6 +81,19 @@ export class NowPlaying extends LitElement {
|
|||||||
|
|
||||||
private reduceMotionQuery?: MediaQueryList;
|
private reduceMotionQuery?: MediaQueryList;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Phone width, from the shell's own breakpoint.
|
||||||
|
*
|
||||||
|
* This is in JS rather than in the stylesheet because what changes
|
||||||
|
* is the *content*, not its appearance: the title, artist and
|
||||||
|
* source render as plain text instead of as links, and no CSS rule
|
||||||
|
* can take a click handler off an element.
|
||||||
|
*/
|
||||||
|
@state()
|
||||||
|
private phone = false;
|
||||||
|
|
||||||
|
private phoneQuery?: MediaQueryList;
|
||||||
|
|
||||||
/** Whether each field is actively mid-scroll (class toggle). */
|
/** Whether each field is actively mid-scroll (class toggle). */
|
||||||
@state()
|
@state()
|
||||||
private titleScrolling = false;
|
private titleScrolling = false;
|
||||||
@@ -341,6 +355,12 @@ export class NowPlaying extends LitElement {
|
|||||||
this.reduceMotion = this.reduceMotionQuery?.matches ?? false;
|
this.reduceMotion = this.reduceMotionQuery?.matches ?? false;
|
||||||
this.reduceMotionQuery?.addEventListener('change', this.handleReduceMotionChange);
|
this.reduceMotionQuery?.addEventListener('change', this.handleReduceMotionChange);
|
||||||
|
|
||||||
|
// Same reasoning as above: looked up here, not at module load,
|
||||||
|
// so a test can install its own matchMedia first.
|
||||||
|
this.phoneQuery = window.matchMedia?.(PHONE_QUERY);
|
||||||
|
this.phone = this.phoneQuery?.matches ?? false;
|
||||||
|
this.phoneQuery?.addEventListener('change', this.handlePhoneChange);
|
||||||
|
|
||||||
this.resizeObserver = new ResizeObserver(() => {
|
this.resizeObserver = new ResizeObserver(() => {
|
||||||
this.geometryDirty = true;
|
this.geometryDirty = true;
|
||||||
this.requestUpdate();
|
this.requestUpdate();
|
||||||
@@ -364,6 +384,7 @@ export class NowPlaying extends LitElement {
|
|||||||
this.attachDragListeners(false);
|
this.attachDragListeners(false);
|
||||||
window.removeEventListener(SCROLL_CHANGE_EVENT, this.handleScrollModeEvent);
|
window.removeEventListener(SCROLL_CHANGE_EVENT, this.handleScrollModeEvent);
|
||||||
this.reduceMotionQuery?.removeEventListener('change', this.handleReduceMotionChange);
|
this.reduceMotionQuery?.removeEventListener('change', this.handleReduceMotionChange);
|
||||||
|
this.phoneQuery?.removeEventListener('change', this.handlePhoneChange);
|
||||||
this.resizeObserver?.disconnect();
|
this.resizeObserver?.disconnect();
|
||||||
this.stopScrollCycle('title');
|
this.stopScrollCycle('title');
|
||||||
this.stopScrollCycle('artist');
|
this.stopScrollCycle('artist');
|
||||||
@@ -488,7 +509,7 @@ export class NowPlaying extends LitElement {
|
|||||||
@mouseleave=${this.handleTitleMouseLeave}
|
@mouseleave=${this.handleTitleMouseLeave}
|
||||||
@transitionend=${() => this.onScrollCycleEnd('title')}
|
@transitionend=${() => this.onScrollCycleEnd('title')}
|
||||||
>
|
>
|
||||||
<span class="scroll-content">${trackLink(track.title, track.album, track.releaseGroupMbid, track.recordingMbid) || track.title}</span>
|
<span class="scroll-content">${this.phone ? track.title : trackLink(track.title, track.album, track.releaseGroupMbid, track.recordingMbid) || track.title}</span>
|
||||||
</span>
|
</span>
|
||||||
<span
|
<span
|
||||||
class="track-artist ${artistScrolling ? 'will-scroll' : ''} ${this.artistScrolling ? 'scrolling' : ''}"
|
class="track-artist ${artistScrolling ? 'will-scroll' : ''} ${this.artistScrolling ? 'scrolling' : ''}"
|
||||||
@@ -498,14 +519,15 @@ export class NowPlaying extends LitElement {
|
|||||||
@mouseleave=${this.handleArtistMouseLeave}
|
@mouseleave=${this.handleArtistMouseLeave}
|
||||||
@transitionend=${() => this.onScrollCycleEnd('artist')}
|
@transitionend=${() => this.onScrollCycleEnd('artist')}
|
||||||
>
|
>
|
||||||
<span class="scroll-content">${creditLink(creditStore.credits(track.recordingMbid), track.artist, track.artistMbid) || 'Unknown Artist'}</span>
|
<span class="scroll-content">${this.phone ? track.artist || 'Unknown Artist' : creditLink(creditStore.credits(track.recordingMbid), track.artist, track.artistMbid) || 'Unknown Artist'}</span>
|
||||||
</span>
|
</span>
|
||||||
${describeQueueSource(this.queue.source)
|
${describeQueueSource(this.queue.source)
|
||||||
? html`
|
? html`
|
||||||
<span
|
<span
|
||||||
class="track-source ${isQueueSourceNavigable(this.queue.source) ? 'navigable' : ''}"
|
class="track-source ${!this.phone && isQueueSourceNavigable(this.queue.source) ? 'navigable' : ''}"
|
||||||
data-testid="now-playing-source"
|
data-testid="now-playing-source"
|
||||||
@click=${(e: MouseEvent) => {
|
@click=${(e: MouseEvent) => {
|
||||||
|
if (this.phone) return;
|
||||||
if (!isQueueSourceNavigable(this.queue.source)) return;
|
if (!isQueueSourceNavigable(this.queue.source)) return;
|
||||||
navigateToQueueSource(
|
navigateToQueueSource(
|
||||||
e.currentTarget as EventTarget,
|
e.currentTarget as EventTarget,
|
||||||
@@ -571,6 +593,10 @@ export class NowPlaying extends LitElement {
|
|||||||
this.reduceMotion = e.matches;
|
this.reduceMotion = e.matches;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
private handlePhoneChange = (e: MediaQueryListEvent): void => {
|
||||||
|
this.phone = e.matches;
|
||||||
|
};
|
||||||
|
|
||||||
private shouldScroll(field: 'title' | 'artist'): boolean {
|
private shouldScroll(field: 'title' | 'artist'): boolean {
|
||||||
const overflows = field === 'title' ? this.titleOverflows : this.artistOverflows;
|
const overflows = field === 'title' ? this.titleOverflows : this.artistOverflows;
|
||||||
|
|
||||||
@@ -606,6 +632,12 @@ export class NowPlaying extends LitElement {
|
|||||||
track?.artist ?? '',
|
track?.artist ?? '',
|
||||||
this.shouldScroll('title') ? '1' : '0',
|
this.shouldScroll('title') ? '1' : '0',
|
||||||
this.shouldScroll('artist') ? '1' : '0',
|
this.shouldScroll('artist') ? '1' : '0',
|
||||||
|
// Crossing the breakpoint swaps a link for a bare string,
|
||||||
|
// and a link is not guaranteed to measure the same as the
|
||||||
|
// text inside it. The marquee travels a distance read from
|
||||||
|
// that measurement, so this belongs in the key even though
|
||||||
|
// the words are identical either side.
|
||||||
|
this.phone ? '1' : '0',
|
||||||
].join('\u0000');
|
].join('\u0000');
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -11,6 +11,7 @@ import {
|
|||||||
import { SelectionController } from '@utils/selection-controller';
|
import { SelectionController } from '@utils/selection-controller';
|
||||||
import type { SelectionHost } from '@utils/selection-controller';
|
import type { SelectionHost } from '@utils/selection-controller';
|
||||||
import { ViewLifecycleMixin } from '@utils/view-lifecycle';
|
import { ViewLifecycleMixin } from '@utils/view-lifecycle';
|
||||||
|
import { PHONE_QUERY } from '@utils/breakpoints';
|
||||||
import {
|
import {
|
||||||
ContextMenuController,
|
ContextMenuController,
|
||||||
contextMenuStyles,
|
contextMenuStyles,
|
||||||
@@ -105,9 +106,6 @@ const ROW_CHROME_WIDTH =
|
|||||||
const ROW_HEIGHT = 33;
|
const ROW_HEIGHT = 33;
|
||||||
const PHONE_ROW_HEIGHT = 52;
|
const PHONE_ROW_HEIGHT = 52;
|
||||||
|
|
||||||
/** The shell's phone breakpoint, as `index.css` and every component
|
|
||||||
* stylesheet spells it. */
|
|
||||||
const PHONE_QUERY = '(max-width: 599px)';
|
|
||||||
|
|
||||||
// Inline SVG paths for favorite icons — eliminates wa-icon shadow DOM
|
// Inline SVG paths for favorite icons — eliminates wa-icon shadow DOM
|
||||||
// overhead (30-50 shadow roots during scroll). Font Awesome 6 paths.
|
// overhead (30-50 shadow roots during scroll). Font Awesome 6 paths.
|
||||||
|
|||||||
@@ -0,0 +1,23 @@
|
|||||||
|
/**
|
||||||
|
* The shell's breakpoints, where JavaScript has to agree with CSS.
|
||||||
|
*
|
||||||
|
* A media query inside a shadow root is answered by the viewport, so a
|
||||||
|
* component normally states what it drops at phone width in its own
|
||||||
|
* stylesheet and needs nothing from here. This exists for the cases
|
||||||
|
* where the decision is not a style: `track-list` computes its grid in
|
||||||
|
* JS from the host width, and `now-playing` renders *different content*
|
||||||
|
* on a phone — a plain string instead of a link — which no stylesheet
|
||||||
|
* can express.
|
||||||
|
*
|
||||||
|
* One breakpoint, several expressions of it. It was a private const in
|
||||||
|
* track-list.ts when there was one; a second reader is where a copy
|
||||||
|
* would start drifting from index.css.
|
||||||
|
*/
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Phone width. 600px rather than the sidebar's 900px because 900 is a
|
||||||
|
* laptop: the answer there is a narrower sidebar, which is still a
|
||||||
|
* sidebar. Below this the shell drops the sidebar column entirely and
|
||||||
|
* bottom-nav takes over.
|
||||||
|
*/
|
||||||
|
export const PHONE_QUERY = '(max-width: 599px)';
|
||||||
@@ -0,0 +1,85 @@
|
|||||||
|
/**
|
||||||
|
* A hover affordance is gated on the device having hover.
|
||||||
|
*
|
||||||
|
* The home page's cover cards reveal a play button on :hover. A touch
|
||||||
|
* long-press synthesises a hover state in the WebView, so on a phone
|
||||||
|
* that button flashed into view during the 500ms hold that
|
||||||
|
* utils/long-press.ts is measuring for a context menu — a control
|
||||||
|
* appearing because the user was reaching for a different one.
|
||||||
|
*
|
||||||
|
* This is asserted against the *parsed stylesheet* rather than by
|
||||||
|
* emulating a touch device, and that is a limitation worth stating
|
||||||
|
* rather than hiding. CDP's Emulation.setEmulatedMedia does not reach
|
||||||
|
* this tier's iframe — matchMedia still answers `hover: hover` after it
|
||||||
|
* is set — so there is no way here to render the component as a phone
|
||||||
|
* would and read the computed style. What can be checked is the shape
|
||||||
|
* the browser actually built from the css`` literal: that the reveal
|
||||||
|
* lives inside a hover media query and that the default is display:none.
|
||||||
|
*
|
||||||
|
* Which is the regression worth catching anyway. The failure mode is
|
||||||
|
* someone hoisting the rule back out of the query for a one-line tidy —
|
||||||
|
* a change nothing renders differently on a desktop, so every other
|
||||||
|
* assertion in this repo passes and the phone silently regresses.
|
||||||
|
*/
|
||||||
|
import { describe, expect, it } from 'vitest';
|
||||||
|
|
||||||
|
import '@components/home-view/home-view';
|
||||||
|
import { fixture } from '@test/support/render';
|
||||||
|
|
||||||
|
/** Every rule in the element's own adopted stylesheets, flattened. */
|
||||||
|
function rulesOf(host: Element): { text: string; condition: string | null }[] {
|
||||||
|
const sheets = host.shadowRoot?.adoptedStyleSheets ?? [];
|
||||||
|
const out: { text: string; condition: string | null }[] = [];
|
||||||
|
|
||||||
|
for (const sheet of sheets) {
|
||||||
|
for (const rule of Array.from(sheet.cssRules)) {
|
||||||
|
if (rule instanceof CSSMediaRule) {
|
||||||
|
for (const inner of Array.from(rule.cssRules)) {
|
||||||
|
out.push({ text: inner.cssText, condition: rule.conditionText });
|
||||||
|
}
|
||||||
|
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
out.push({ text: rule.cssText, condition: null });
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return out;
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('the home card play button', () => {
|
||||||
|
it('reveals itself only where the device has hover', async () => {
|
||||||
|
const el = await fixture('home-view', {});
|
||||||
|
const rules = rulesOf(el);
|
||||||
|
|
||||||
|
// The sweep is worth nothing if it read no rules at all — the same
|
||||||
|
// first assertion icon-language.test.ts makes for the same reason.
|
||||||
|
expect(rules.length).toBeGreaterThan(0);
|
||||||
|
|
||||||
|
const reveals = rules.filter(
|
||||||
|
(r) => r.text.includes('.play') && /opacity:\s*1/.test(r.text),
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(reveals.length).toBeGreaterThan(0);
|
||||||
|
|
||||||
|
for (const rule of reveals) {
|
||||||
|
expect(rule.condition).toMatch(/hover:\s*hover/);
|
||||||
|
expect(rule.condition).toMatch(/pointer:\s*fine/);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it('is display:none rather than transparent where it is absent', async () => {
|
||||||
|
const el = await fixture('home-view', {});
|
||||||
|
|
||||||
|
// opacity:0 alone would leave a button that still takes taps and is
|
||||||
|
// still in the accessibility tree, so a phone would keep the hit
|
||||||
|
// area for a control it can never see.
|
||||||
|
const unconditional = rulesOf(el).filter(
|
||||||
|
(r) => r.condition === null && r.text.startsWith('.play'),
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(unconditional.length).toBeGreaterThan(0);
|
||||||
|
expect(unconditional.some((r) => /display:\s*none/.test(r.text))).toBe(true);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -0,0 +1,166 @@
|
|||||||
|
/**
|
||||||
|
* The mini player's links are a desktop affordance.
|
||||||
|
*
|
||||||
|
* `utils/explore-link.ts` makes every track and artist name navigate,
|
||||||
|
* and `utils/queue-source-link.ts` makes "Playing from X" navigate — in
|
||||||
|
* the bottom bar those are a few characters of text at a font size
|
||||||
|
* chosen for a bar, which is not a touch target. Worse, explore-link
|
||||||
|
* holds the navigation for one double-click interval and drops it if a
|
||||||
|
* second click arrives: a gesture that exists so double-clicking a row
|
||||||
|
* can play it, and which means nothing at all on touch.
|
||||||
|
*
|
||||||
|
* So below the shell's phone breakpoint the three render as plain text
|
||||||
|
* and the whole bar's cover art opens the full-screen Now Playing view,
|
||||||
|
* which is where the links live.
|
||||||
|
*
|
||||||
|
* The breakpoint is stubbed rather than emulated for the reason
|
||||||
|
* track-list-phone.test.ts states: this tier's viewport is fixed at
|
||||||
|
* 1280x800 by the runner, and the component reads matchMedia in
|
||||||
|
* connectedCallback precisely so a test can answer it first.
|
||||||
|
*/
|
||||||
|
import { describe, expect, it, beforeEach } from 'vitest';
|
||||||
|
|
||||||
|
import '@components/now-playing/now-playing';
|
||||||
|
import { Events } from '../../src/events';
|
||||||
|
import { emit, flush } from '@test/support/harness';
|
||||||
|
import { fixture, shadow, shadowAll, text } from '@test/support/render';
|
||||||
|
import type { TrackInfo } from '@store/player-store';
|
||||||
|
import type { QueueTrack } from '@store/queue-store';
|
||||||
|
|
||||||
|
const TRACK: TrackInfo = {
|
||||||
|
fileName: 'ashes.mp3',
|
||||||
|
filePath: '/music/ashes.mp3',
|
||||||
|
trackLength: 215,
|
||||||
|
seekPosition: 0,
|
||||||
|
state: 'playing',
|
||||||
|
title: 'Ashes to Ashes',
|
||||||
|
artist: 'David Bowie',
|
||||||
|
album: 'Scary Monsters',
|
||||||
|
coverArt: '',
|
||||||
|
coverArtSmall: '',
|
||||||
|
coverArtMedium: '',
|
||||||
|
coverArtLarge: '',
|
||||||
|
trackChangeId: 1,
|
||||||
|
artistMbid: '',
|
||||||
|
releaseGroupMbid: '',
|
||||||
|
recordingMbid: '',
|
||||||
|
};
|
||||||
|
|
||||||
|
function queueTrack(n: number, title: string): QueueTrack {
|
||||||
|
return {
|
||||||
|
id: n,
|
||||||
|
audioFileId: n,
|
||||||
|
filePath: `/music/${n}.mp3`,
|
||||||
|
position: n,
|
||||||
|
title,
|
||||||
|
artist: 'David Bowie',
|
||||||
|
album: 'Scary Monsters',
|
||||||
|
coverArtPath: '',
|
||||||
|
artistMbid: '',
|
||||||
|
releaseGroupMbid: '',
|
||||||
|
recordingMbid: '',
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Mount the bar with the phone breakpoint answering `matches`. */
|
||||||
|
async function mountAt(phone: boolean) {
|
||||||
|
const real = window.matchMedia.bind(window);
|
||||||
|
|
||||||
|
window.matchMedia = ((q: string) =>
|
||||||
|
q.includes('max-width: 599px')
|
||||||
|
? {
|
||||||
|
matches: phone,
|
||||||
|
media: q,
|
||||||
|
addEventListener() {},
|
||||||
|
removeEventListener() {},
|
||||||
|
}
|
||||||
|
: real(q)) as typeof window.matchMedia;
|
||||||
|
|
||||||
|
try {
|
||||||
|
const el = await fixture('now-playing');
|
||||||
|
|
||||||
|
emit(Events.TrackChanged, { ...TRACK, trackChangeId: 20 });
|
||||||
|
emit(Events.QueueChanged, {
|
||||||
|
tracks: [queueTrack(1, 'Ashes to Ashes')],
|
||||||
|
currentIndex: 0,
|
||||||
|
source: { type: 'album', id: 7, label: 'Scary Monsters' },
|
||||||
|
});
|
||||||
|
await flush();
|
||||||
|
await el.updateComplete;
|
||||||
|
|
||||||
|
return el;
|
||||||
|
} finally {
|
||||||
|
window.matchMedia = real;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('the mini player on a phone', () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
emit(Events.TrackChanged, { ...TRACK, trackChangeId: 1 });
|
||||||
|
});
|
||||||
|
|
||||||
|
it('renders the title and artist as plain text', async () => {
|
||||||
|
const el = await mountAt(true);
|
||||||
|
|
||||||
|
expect(shadowAll(el, '.explore-link').length).toBe(0);
|
||||||
|
|
||||||
|
// The words are unchanged — this is about what they are, not about
|
||||||
|
// hiding them. A fix that dropped the text would pass an assertion
|
||||||
|
// about links alone.
|
||||||
|
expect(text(el, '[data-testid="now-playing-title"]')).toContain(
|
||||||
|
'Ashes to Ashes',
|
||||||
|
);
|
||||||
|
expect(text(el, '[data-testid="now-playing-artist"]')).toContain(
|
||||||
|
'David Bowie',
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does not navigate from the source line', async () => {
|
||||||
|
const el = await mountAt(true);
|
||||||
|
const source = shadow<HTMLElement>(el, '[data-testid="now-playing-source"]');
|
||||||
|
|
||||||
|
expect(source?.classList.contains('navigable')).toBe(false);
|
||||||
|
|
||||||
|
let navigated = false;
|
||||||
|
el.addEventListener('navigate', () => {
|
||||||
|
navigated = true;
|
||||||
|
});
|
||||||
|
|
||||||
|
source?.click();
|
||||||
|
|
||||||
|
expect(navigated).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('still says where the queue came from', async () => {
|
||||||
|
const el = await mountAt(true);
|
||||||
|
|
||||||
|
// Dropping the *link* is the change; dropping the information would
|
||||||
|
// be a different and worse one.
|
||||||
|
expect(text(el, '[data-testid="now-playing-source"]')).toBe(
|
||||||
|
'Playing from Scary Monsters',
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('leaves the desktop bar exactly as it was', async () => {
|
||||||
|
const el = await mountAt(false);
|
||||||
|
|
||||||
|
expect(shadowAll(el, '.explore-link').length).toBeGreaterThan(0);
|
||||||
|
|
||||||
|
const source = shadow<HTMLElement>(el, '[data-testid="now-playing-source"]');
|
||||||
|
|
||||||
|
expect(source?.classList.contains('navigable')).toBe(true);
|
||||||
|
|
||||||
|
let detail: unknown;
|
||||||
|
el.addEventListener('navigate', (e) => {
|
||||||
|
detail = (e as CustomEvent).detail;
|
||||||
|
});
|
||||||
|
|
||||||
|
source?.click();
|
||||||
|
|
||||||
|
expect(detail).toEqual({
|
||||||
|
view: 'explore-album-details',
|
||||||
|
localAlbumId: 7,
|
||||||
|
albumName: 'Scary Monsters',
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
+16
-12
@@ -20,19 +20,14 @@ pre-commit:
|
|||||||
glob: "*.go"
|
glob: "*.go"
|
||||||
run: go tool golangci-lint run --timeout 5m ./...
|
run: go tool golangci-lint run --timeout 5m ./...
|
||||||
|
|
||||||
|
# Snapshots the tree either side of the generators and reports only
|
||||||
|
# what moved across them. This used to be `go generate` plus a bare
|
||||||
|
# `git diff --name-only`, which is the *whole unstaged worktree* — so
|
||||||
|
# any unrelated edit sitting there was reported as stale generated
|
||||||
|
# code, and `make generate` then fixed nothing. See the script.
|
||||||
codegen-check:
|
codegen-check:
|
||||||
glob: "*.{go,sql,templ}"
|
glob: "*.{go,sql,templ}"
|
||||||
run: |
|
run: ./scripts/codegen-check.sh
|
||||||
go generate ./...
|
|
||||||
if [ -n "$(git diff --name-only)" ]; then
|
|
||||||
echo "Generated code is out of date. Run 'make generate' and stage the changes."
|
|
||||||
# --no-pager, or this blocks forever on `less` waiting for a
|
|
||||||
# keypress that a hook run without a tty will never get: the
|
|
||||||
# commit hangs at exactly the moment it is trying to tell you
|
|
||||||
# why it failed.
|
|
||||||
git --no-pager diff --stat
|
|
||||||
exit 1
|
|
||||||
fi
|
|
||||||
|
|
||||||
# frontend/bindings is generated by `wails3`, not `go generate`, so
|
# frontend/bindings is generated by `wails3`, not `go generate`, so
|
||||||
# the check above does not cover it. ~3.5s warm, ~20s on a cold
|
# the check above does not cover it. ~3.5s warm, ~20s on a cold
|
||||||
@@ -63,8 +58,17 @@ pre-commit:
|
|||||||
root: "frontend/"
|
root: "frontend/"
|
||||||
run: node scripts/check-css-literals.mjs
|
run: node scripts/check-css-literals.mjs
|
||||||
|
|
||||||
|
# Deliberately sequential, unlike pre-commit. `go test -race`
|
||||||
|
# saturates every core for the better part of a minute and the UI tier
|
||||||
|
# is a real browser with wall-clock timeouts, so run together the
|
||||||
|
# browser loses: setup took 106s inside the hook against 63s
|
||||||
|
# standalone, and a different suite failed each time -- three suites
|
||||||
|
# failing to fetch setup.ts from Vitest's own dev server on one run, a
|
||||||
|
# 15s "did not mount itself" on the next, against a suite that passes
|
||||||
|
# 898/898 on its own. A gate that fails at random is not a gate. The
|
||||||
|
# ~15s saved is not worth it.
|
||||||
pre-push:
|
pre-push:
|
||||||
parallel: true
|
parallel: false
|
||||||
commands:
|
commands:
|
||||||
go-test:
|
go-test:
|
||||||
glob: "*.go"
|
glob: "*.go"
|
||||||
|
|||||||
Executable
+81
@@ -0,0 +1,81 @@
|
|||||||
|
#!/usr/bin/env bash
|
||||||
|
#
|
||||||
|
# Fails when `go generate ./...` would change something that is not staged.
|
||||||
|
#
|
||||||
|
# The obvious spelling of this is `go generate && git diff --name-only`,
|
||||||
|
# which is what the hook used to be, and it answers the wrong question:
|
||||||
|
# that diff is the *whole unstaged worktree*, so any unrelated edit — a
|
||||||
|
# note, a plan document, the next commit's files sitting there while this
|
||||||
|
# one lands — was reported as
|
||||||
|
#
|
||||||
|
# Generated code is out of date. Run 'make generate' and stage the changes.
|
||||||
|
#
|
||||||
|
# Running `make generate` then does nothing, because nothing generated is
|
||||||
|
# stale, and the message sends you looking for a codegen problem that does
|
||||||
|
# not exist. Splitting one piece of work into several commits is exactly
|
||||||
|
# the shape that triggers it, so the workaround was a constraint on commit
|
||||||
|
# order for no real reason.
|
||||||
|
#
|
||||||
|
# So the tree is snapshotted either side of the generators and only what
|
||||||
|
# *moved across them* is reported. That is deliberately not a list of
|
||||||
|
# generated paths: sqlcgen, `*_templ.go` and `frontend/src/events.ts` are
|
||||||
|
# today's answer, a fourth generator is one `//go:generate` line away, and
|
||||||
|
# a path list is a second place to remember it — the same reasoning that
|
||||||
|
# keeps staleshape.go parsing sql/schemas/ rather than restating it.
|
||||||
|
#
|
||||||
|
# Content, not names: a generated file that is *already* dirty and is then
|
||||||
|
# rewritten further keeps its name in both snapshots and would otherwise
|
||||||
|
# slip through.
|
||||||
|
|
||||||
|
set -euo pipefail
|
||||||
|
|
||||||
|
cd "$(dirname "$0")/.."
|
||||||
|
|
||||||
|
# name + worktree blob hash for every file that differs from the index.
|
||||||
|
# A file listed but absent (a deletion) hashes as "gone" rather than
|
||||||
|
# aborting the pipeline.
|
||||||
|
snapshot() {
|
||||||
|
git diff --name-only | while IFS= read -r f; do
|
||||||
|
if [ -f "$f" ]; then
|
||||||
|
printf '%s %s\n' "$f" "$(git hash-object -- "$f")"
|
||||||
|
else
|
||||||
|
printf '%s gone\n' "$f"
|
||||||
|
fi
|
||||||
|
done
|
||||||
|
}
|
||||||
|
|
||||||
|
# A brand-new generated file is not in either diff, because it is not
|
||||||
|
# tracked at all — the same blind spot bindings-check.sh names. Both
|
||||||
|
# snapshots are taken before the generators run.
|
||||||
|
before="$(snapshot)"
|
||||||
|
before_untracked="$(git ls-files --others --exclude-standard)"
|
||||||
|
|
||||||
|
go generate ./...
|
||||||
|
|
||||||
|
after="$(snapshot)"
|
||||||
|
after_untracked="$(git ls-files --others --exclude-standard)"
|
||||||
|
|
||||||
|
# Symmetric difference, and the symmetry is the whole point. Generation
|
||||||
|
# can push a file *into* the unstaged set (it was current, now it is not)
|
||||||
|
# or *out* of it (someone hand-edited generated output and the generator
|
||||||
|
# put it back) — and the second is stale generated code just as much as
|
||||||
|
# the first. Comparing one direction only reports "current" for it,
|
||||||
|
# which is the failure this script was written to stop.
|
||||||
|
moved="$(comm -3 <(printf '%s\n' "$before" | sort) <(printf '%s\n' "$after" | sort) |
|
||||||
|
cut -d' ' -f1 | tr -d '\t' | sort -u | grep -v '^$' || true)"
|
||||||
|
|
||||||
|
if [ -n "$moved" ]; then
|
||||||
|
echo "codegen-check: generated code is out of date." >&2
|
||||||
|
echo "Run 'make generate' and stage:" >&2
|
||||||
|
printf ' %s\n' $moved >&2
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
|
||||||
|
if [ "$after_untracked" != "$before_untracked" ]; then
|
||||||
|
echo "codegen-check: generation produced new files. Stage them:" >&2
|
||||||
|
comm -13 <(printf '%s\n' "$before_untracked" | sort) \
|
||||||
|
<(printf '%s\n' "$after_untracked" | sort) >&2
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
|
||||||
|
echo "codegen-check: generated code is current"
|
||||||
@@ -97,6 +97,55 @@ if [ -f "$PID_FILE" ] && kill -0 "$(cat "$PID_FILE")" 2>/dev/null; then
|
|||||||
fi
|
fi
|
||||||
rm -f "$PID_FILE"
|
rm -f "$PID_FILE"
|
||||||
|
|
||||||
|
# ── Refuse to inherit somebody else's port ───────────────────────────
|
||||||
|
# The PID check above only knows about *this* worktree: `make dev-stop`
|
||||||
|
# kills the pid in this .dev/app.pid and nothing else. Several worktrees
|
||||||
|
# of this repo share the default port, so an app orphaned by a deleted
|
||||||
|
# worktree goes on listening with nothing left to stop it.
|
||||||
|
#
|
||||||
|
# Without this check the new app starts, fails to bind, exits — and every
|
||||||
|
# curl and playwright-cli call afterwards goes to the *other* process, so
|
||||||
|
# the harness reports facts about an app nobody asked for. That is not a
|
||||||
|
# quiet wrongness either: it presented as
|
||||||
|
# "no such table: libraries" against a freshly created YJ_HOME, which
|
||||||
|
# reads exactly like applySchema or staleshape.go having gone wrong and
|
||||||
|
# is a frightening place to start looking.
|
||||||
|
#
|
||||||
|
# The startup wait below cannot catch it, because the health check is
|
||||||
|
# satisfied by *any* app on the port — which is precisely the failure.
|
||||||
|
# So it is refused here, before anything is launched, rather than warned
|
||||||
|
# about. --port already exists for the legitimate second-app case.
|
||||||
|
port_holder() {
|
||||||
|
command -v ss >/dev/null || return 0
|
||||||
|
ss -lptn "sport = :$PORT" 2>/dev/null | grep -oP 'pid=\K[0-9]+' | head -n 1
|
||||||
|
}
|
||||||
|
|
||||||
|
if curl -sf -o /dev/null --max-time 2 "http://localhost:$PORT/" ||
|
||||||
|
[ -n "$(port_holder)" ]; then
|
||||||
|
holder="$(port_holder)"
|
||||||
|
echo "dev-headless: :$PORT is already in use; refusing to start" >&2
|
||||||
|
if [ -n "$holder" ]; then
|
||||||
|
# /proc/<pid>/cwd names the checkout it belongs to, and says
|
||||||
|
# "(deleted)" for the orphaned-worktree case that is the whole
|
||||||
|
# reason this is worth a check.
|
||||||
|
cwd="$(readlink "/proc/$holder/cwd" 2>/dev/null || echo unknown)"
|
||||||
|
cmd="$(tr '\0' ' ' <"/proc/$holder/cmdline" 2>/dev/null || echo unknown)"
|
||||||
|
echo " pid $holder ($cmd)" >&2
|
||||||
|
echo " cwd $cwd" >&2
|
||||||
|
# The PID-file check above has already passed, so whatever this
|
||||||
|
# is, `make dev-stop` does not know about it — saying otherwise
|
||||||
|
# sends you to a command that will report success and change
|
||||||
|
# nothing. Never `pkill -f` here either: the pattern would
|
||||||
|
# match this script's own command line.
|
||||||
|
echo " 'make dev-stop' will not touch it (it is not in" >&2
|
||||||
|
echo " ${PID_FILE#"$REPO_ROOT"/}): kill $holder, or pass --port." >&2
|
||||||
|
else
|
||||||
|
echo " The holder could not be identified (no ss, or it belongs" >&2
|
||||||
|
echo " to another user). Try: ss -lptn 'sport = :$PORT'" >&2
|
||||||
|
fi
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
|
||||||
# ── Choose the YJ_HOME ───────────────────────────────────────────────
|
# ── Choose the YJ_HOME ───────────────────────────────────────────────
|
||||||
# A seed is a YJ_HOME that a previous run of the app produced, tarred
|
# A seed is a YJ_HOME that a previous run of the app produced, tarred
|
||||||
# up (see scripts/seed-sandbox.sh). Restoring it means starting *in*
|
# up (see scripts/seed-sandbox.sh). Restoring it means starting *in*
|
||||||
@@ -200,6 +249,20 @@ until curl -sf -o /dev/null "http://localhost:$PORT/"; do
|
|||||||
sleep 0.25
|
sleep 0.25
|
||||||
done
|
done
|
||||||
|
|
||||||
|
# The loop above exits on the first answer from the port, and "something
|
||||||
|
# answered" is not "the app we started answered". The pre-launch guard
|
||||||
|
# makes that unlikely rather than impossible — a race, or a listener
|
||||||
|
# started in between — and the check is one signal, so it is worth making
|
||||||
|
# here too. An empty log beside a dead pid is the "it exited immediately
|
||||||
|
# and nothing said so" case that the original report spent its time on.
|
||||||
|
if ! kill -0 "$APP_PID" 2>/dev/null; then
|
||||||
|
echo "dev-headless: :$PORT answered, but the app we started (pid" >&2
|
||||||
|
echo " $APP_PID) is gone — something else holds the port." >&2
|
||||||
|
tail -n 30 "$LOG_FILE" >&2
|
||||||
|
rm -f "$PID_FILE"
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
|
||||||
cat <<EOF
|
cat <<EOF
|
||||||
dev-headless: up
|
dev-headless: up
|
||||||
url http://localhost:$PORT
|
url http://localhost:$PORT
|
||||||
|
|||||||
+27
-3
@@ -38,8 +38,10 @@
|
|||||||
# Where a body is taken and no --body-file is given, it is read from stdin.
|
# Where a body is taken and no --body-file is given, it is read from stdin.
|
||||||
#
|
#
|
||||||
# Environment:
|
# Environment:
|
||||||
# GITEA_TOKEN a PAT with write:issue (plus write:repository and read:user,
|
# GITEA_TOKEN a PAT with write:issue. `claim` and `mine` additionally
|
||||||
# which the rest of this repo's tooling reaches for)
|
# need to know your username: set GITEA_USER, or give the
|
||||||
|
# token read:user and it is looked up.
|
||||||
|
# GITEA_USER your Gitea login. Optional; see above.
|
||||||
# GITEA_URL defaults to https://git.ljones.me
|
# GITEA_URL defaults to https://git.ljones.me
|
||||||
# GITEA_REPO defaults to yonlu/yellowjacket
|
# GITEA_REPO defaults to yonlu/yellowjacket
|
||||||
set -euo pipefail
|
set -euo pipefail
|
||||||
@@ -81,7 +83,29 @@ read_body() {
|
|||||||
if [ "$file" = "-" ]; then cat; else cat "$file"; fi
|
if [ "$file" = "-" ]; then cat; else cat "$file"; fi
|
||||||
}
|
}
|
||||||
|
|
||||||
me() { curl -sS -H "Authorization: token $GITEA_TOKEN" "$server/api/v1/user" | python3 "$py" login; }
|
# The one lookup in this script that needs a scope beyond write:issue.
|
||||||
|
# `GET /user` requires read:user, and it is reached for exactly two reasons:
|
||||||
|
# to name the assignee in `claim`, and to filter in `mine`. A token scoped to
|
||||||
|
# the work this script does — write:issue — therefore failed at `claim`, which
|
||||||
|
# is the one step the workflow requires before the first edit, so the whole
|
||||||
|
# documented process was blocked by its own tooling.
|
||||||
|
#
|
||||||
|
# GITEA_USER short-circuits it, which is what lets a least-privilege token do
|
||||||
|
# the job. The lookup stays as the fallback because it is right when the
|
||||||
|
# scope is there and needs no setup at all.
|
||||||
|
me() {
|
||||||
|
if [ -n "${GITEA_USER:-}" ]; then
|
||||||
|
printf '%s' "$GITEA_USER"
|
||||||
|
return
|
||||||
|
fi
|
||||||
|
curl -sS -H "Authorization: token $GITEA_TOKEN" "$server/api/v1/user" |
|
||||||
|
python3 "$py" login ||
|
||||||
|
{
|
||||||
|
echo "issue.sh: could not resolve your username. Set GITEA_USER, or" >&2
|
||||||
|
echo "issue.sh: re-issue GITEA_TOKEN with read:user." >&2
|
||||||
|
exit 1
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
label_id() { call GET "/labels?limit=100" | python3 "$py" label-id "$1"; }
|
label_id() { call GET "/labels?limit=100" | python3 "$py" label-id "$1"; }
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user