Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
0d331666d6 | ||
|
|
6a5a3c33dc | ||
|
|
1668b9e0d2 | ||
|
|
ec64dbded0 | ||
|
|
dad852a8a0 | ||
|
|
30c6b665f1 | ||
|
|
d034d6e571 | ||
|
|
25ea1f3511 |
@@ -40,7 +40,8 @@ WHERE id = ? AND (mbid IS NULL OR mbid = '');
|
||||
-- name: GetAlbumsWithPendingReleaseMBID :many
|
||||
SELECT id, pending_release_mbid FROM albums
|
||||
WHERE pending_release_mbid IS NOT NULL AND pending_release_mbid != ''
|
||||
AND (mbid IS NULL OR mbid = '');
|
||||
AND (mbid IS NULL OR mbid = '')
|
||||
LIMIT ?;
|
||||
|
||||
-- name: DeleteAlbum :exec
|
||||
DELETE FROM albums WHERE id = ?;
|
||||
|
||||
@@ -331,6 +331,7 @@ const getAlbumsWithPendingReleaseMBID = `-- name: GetAlbumsWithPendingReleaseMBI
|
||||
SELECT id, pending_release_mbid FROM albums
|
||||
WHERE pending_release_mbid IS NOT NULL AND pending_release_mbid != ''
|
||||
AND (mbid IS NULL OR mbid = '')
|
||||
LIMIT ?
|
||||
`
|
||||
|
||||
type GetAlbumsWithPendingReleaseMBIDRow struct {
|
||||
@@ -338,8 +339,8 @@ type GetAlbumsWithPendingReleaseMBIDRow struct {
|
||||
PendingReleaseMbid sql.NullString
|
||||
}
|
||||
|
||||
func (q *Queries) GetAlbumsWithPendingReleaseMBID(ctx context.Context) ([]GetAlbumsWithPendingReleaseMBIDRow, error) {
|
||||
rows, err := q.db.QueryContext(ctx, getAlbumsWithPendingReleaseMBID)
|
||||
func (q *Queries) GetAlbumsWithPendingReleaseMBID(ctx context.Context, limit int64) ([]GetAlbumsWithPendingReleaseMBIDRow, error) {
|
||||
rows, err := q.db.QueryContext(ctx, getAlbumsWithPendingReleaseMBID, limit)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
+35
-31
@@ -2,6 +2,7 @@ package explore
|
||||
|
||||
import (
|
||||
"context"
|
||||
"database/sql"
|
||||
"log/slog"
|
||||
"math"
|
||||
"sort"
|
||||
@@ -12,6 +13,7 @@ import (
|
||||
"golang.org/x/sync/singleflight"
|
||||
|
||||
"yellowjacket/backend/database"
|
||||
"yellowjacket/backend/database/sql/sqlcgen"
|
||||
"yellowjacket/backend/events"
|
||||
"yellowjacket/backend/jobs"
|
||||
)
|
||||
@@ -279,37 +281,32 @@ func (e *Service) BackfillReleaseGroupMBIDs() {
|
||||
go e.backfillReleaseGroupMBIDs(e.ctx)
|
||||
}
|
||||
|
||||
func (e *Service) backfillReleaseGroupMBIDs(ctx context.Context) {
|
||||
rows, err := e.db.QueryContext(
|
||||
"SELECT id, pending_release_mbid FROM release_groups "+
|
||||
"WHERE (mbid IS NULL OR mbid = '') "+
|
||||
"AND pending_release_mbid IS NOT NULL AND pending_release_mbid != '' "+
|
||||
"LIMIT ?",
|
||||
releaseGroupMBIDBackfillMaxPerRun,
|
||||
// pendingReleaseMBIDs is the albums this pass has work to do on.
|
||||
//
|
||||
// It is separate from the pass, and returns its error rather than
|
||||
// logging it, so that a test can assert the statement runs against the
|
||||
// real schema. That is not a general preference -- it is this
|
||||
// statement's history: it named `release_groups`, a table plan 013
|
||||
// renamed to `albums`, so it failed on every launch since e7748f1 and
|
||||
// the pass returned quietly having done nothing. A test of the pass
|
||||
// as a whole cannot see that, because a query error and an empty
|
||||
// library are the same early return.
|
||||
func (e *Service) pendingReleaseMBIDs(
|
||||
ctx context.Context,
|
||||
) ([]sqlcgen.GetAlbumsWithPendingReleaseMBIDRow, error) {
|
||||
return e.db.ReadQueries.GetAlbumsWithPendingReleaseMBID(
|
||||
ctx, releaseGroupMBIDBackfillMaxPerRun,
|
||||
)
|
||||
}
|
||||
|
||||
func (e *Service) backfillReleaseGroupMBIDs(ctx context.Context) {
|
||||
pending, err := e.pendingReleaseMBIDs(ctx)
|
||||
if err != nil {
|
||||
e.logger.Warn("release-group mbid backfill: query failed", "error", err)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
type pendingRow struct {
|
||||
id int64
|
||||
releaseMBID string
|
||||
}
|
||||
|
||||
var pending []pendingRow
|
||||
|
||||
for rows.Next() {
|
||||
var p pendingRow
|
||||
|
||||
if err := rows.Scan(&p.id, &p.releaseMBID); err == nil {
|
||||
pending = append(pending, p)
|
||||
}
|
||||
}
|
||||
|
||||
_ = rows.Close()
|
||||
|
||||
if len(pending) == 0 {
|
||||
return
|
||||
}
|
||||
@@ -335,7 +332,7 @@ func (e *Service) backfillReleaseGroupMBIDs(ctx context.Context) {
|
||||
|
||||
job.progress(i, len(pending))
|
||||
|
||||
release, err := e.mb.LookupRelease(ctx, p.releaseMBID)
|
||||
release, err := e.mb.LookupRelease(ctx, p.PendingReleaseMbid.String)
|
||||
if err != nil || release.ReleaseGroupMBID == "" {
|
||||
// Left alone rather than cleared: LookupRelease caches its
|
||||
// answer (success or a release with no group) for 7 days,
|
||||
@@ -344,12 +341,19 @@ func (e *Service) backfillReleaseGroupMBIDs(ctx context.Context) {
|
||||
continue
|
||||
}
|
||||
|
||||
_, err = e.db.ExecContext(
|
||||
"UPDATE release_groups SET mbid = ?, pending_release_mbid = NULL "+
|
||||
"WHERE id = ? AND (mbid IS NULL OR mbid = '')",
|
||||
release.ReleaseGroupMBID, p.id,
|
||||
)
|
||||
if err != nil {
|
||||
// The writer, not ReadQueries: an UPDATE issued on the
|
||||
// query-only pool fails at runtime with "attempt to write a
|
||||
// readonly database".
|
||||
if err := e.db.Queries.ResolveAlbumPendingReleaseMBID(
|
||||
ctx,
|
||||
sqlcgen.ResolveAlbumPendingReleaseMBIDParams{
|
||||
Mbid: sql.NullString{
|
||||
String: release.ReleaseGroupMBID,
|
||||
Valid: true,
|
||||
},
|
||||
ID: p.ID,
|
||||
},
|
||||
); err != nil {
|
||||
e.logger.Warn("release-group mbid backfill: update failed", "error", err)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,250 @@
|
||||
package explore
|
||||
|
||||
import (
|
||||
"database/sql"
|
||||
"log/slog"
|
||||
"strconv"
|
||||
"testing"
|
||||
|
||||
"yellowjacket/backend/database"
|
||||
"yellowjacket/backend/database/sql/sqlcgen"
|
||||
)
|
||||
|
||||
// The release-group MBID backfill queried `release_groups`, a table
|
||||
// plan 013 renamed to `albums`, so it failed on its first statement on
|
||||
// every launch from e7748f1 until #189 -- and the pass swallowed that,
|
||||
// because a query error and an empty library are the same early
|
||||
// return. Nothing noticed for two reasons worth keeping in mind:
|
||||
//
|
||||
// - the statement was **raw SQL**, so sqlc never read it. Every other
|
||||
// statement in the repo was renamed by the same change because sqlc
|
||||
// reads sql/schemas/ and cannot generate against a table that is not
|
||||
// declared. The two sqlc queries this now calls were written by 013
|
||||
// and left uncalled.
|
||||
// - it needs no network and no fixture library to reproduce. The
|
||||
// failure is at prepare time.
|
||||
|
||||
// seedPendingAlbum inserts an album whose files carried a release MBID
|
||||
// but no release-group MBID, which is what `library.updateMBIDs`
|
||||
// leaves behind for this pass to resolve.
|
||||
func seedPendingAlbum(
|
||||
t *testing.T,
|
||||
db *database.DB,
|
||||
name, pendingMBID string,
|
||||
) int64 {
|
||||
t.Helper()
|
||||
|
||||
res, err := db.ExecContext(
|
||||
"INSERT INTO albums (name, artist_credit, pending_release_mbid) "+
|
||||
"VALUES (?, ?, ?)",
|
||||
name, "Test Artist", pendingMBID,
|
||||
)
|
||||
if err != nil {
|
||||
t.Fatalf("insert albums row: %v", err)
|
||||
}
|
||||
|
||||
id, err := res.LastInsertId()
|
||||
if err != nil {
|
||||
t.Fatalf("last insert id: %v", err)
|
||||
}
|
||||
|
||||
return id
|
||||
}
|
||||
|
||||
func newPendingTestService(db *database.DB) *Service {
|
||||
return &Service{db: db, logger: slog.Default()}
|
||||
}
|
||||
|
||||
// TestPendingReleaseMBIDsRunsAgainstTheRealSchema is the regression.
|
||||
//
|
||||
// It asserts the statement *runs*, which is the whole of what was
|
||||
// broken: against the old raw SQL this returns
|
||||
// "no such table: release_groups" rather than a row.
|
||||
func TestPendingReleaseMBIDsRunsAgainstTheRealSchema(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
db := database.NewTestDB(t)
|
||||
e := newPendingTestService(db)
|
||||
|
||||
want := seedPendingAlbum(t, db, "Pending Album", "release-mbid-1")
|
||||
|
||||
pending, err := e.pendingReleaseMBIDs(db.Ctx)
|
||||
if err != nil {
|
||||
t.Fatalf("the backfill's query failed: %v", err)
|
||||
}
|
||||
|
||||
if len(pending) != 1 {
|
||||
t.Fatalf("got %d pending albums, want 1", len(pending))
|
||||
}
|
||||
|
||||
if pending[0].ID != want {
|
||||
t.Errorf("got album id %d, want %d", pending[0].ID, want)
|
||||
}
|
||||
|
||||
if got := pending[0].PendingReleaseMbid.String; got != "release-mbid-1" {
|
||||
t.Errorf("got pending mbid %q, want %q", got, "release-mbid-1")
|
||||
}
|
||||
}
|
||||
|
||||
// TestOnlyUnresolvedAlbumsAreReturned pins the two conditions that make
|
||||
// the pass idempotent, since between them they are what stops it doing
|
||||
// the same MusicBrainz lookups on every launch forever.
|
||||
func TestOnlyUnresolvedAlbumsAreReturned(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
db := database.NewTestDB(t)
|
||||
e := newPendingTestService(db)
|
||||
|
||||
pendingID := seedPendingAlbum(t, db, "Still Pending", "release-mbid-1")
|
||||
|
||||
// Already resolved: it has a real MBID, so there is nothing to
|
||||
// look up even though a marker is still sitting on it.
|
||||
resolved := seedPendingAlbum(t, db, "Already Resolved", "release-mbid-2")
|
||||
if err := db.Queries.SetAlbumMBID(db.Ctx, sqlcgen.SetAlbumMBIDParams{
|
||||
Mbid: sql.NullString{String: "rg-mbid", Valid: true},
|
||||
ID: resolved,
|
||||
}); err != nil {
|
||||
t.Fatalf("set album mbid: %v", err)
|
||||
}
|
||||
|
||||
// Never had a release MBID to resolve in the first place, which is
|
||||
// most of a library.
|
||||
seedPendingAlbum(t, db, "Nothing Pending", "")
|
||||
|
||||
pending, err := e.pendingReleaseMBIDs(db.Ctx)
|
||||
if err != nil {
|
||||
t.Fatalf("the backfill's query failed: %v", err)
|
||||
}
|
||||
|
||||
if len(pending) != 1 || pending[0].ID != pendingID {
|
||||
t.Fatalf(
|
||||
"got %d albums %v, want only the unresolved one (%d)",
|
||||
len(pending), pending, pendingID,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// TestResolvingClearsTheMarker is the other half: once the lookup has
|
||||
// answered, the album must stop being a candidate, or the pass repeats
|
||||
// the same live MusicBrainz call on every launch.
|
||||
func TestResolvingClearsTheMarker(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
db := database.NewTestDB(t)
|
||||
e := newPendingTestService(db)
|
||||
|
||||
id := seedPendingAlbum(t, db, "Pending Album", "release-mbid-1")
|
||||
|
||||
// The writer, deliberately: this is an UPDATE, and the read pool
|
||||
// would refuse it at runtime.
|
||||
if err := db.Queries.ResolveAlbumPendingReleaseMBID(
|
||||
db.Ctx,
|
||||
sqlcgen.ResolveAlbumPendingReleaseMBIDParams{
|
||||
Mbid: sql.NullString{String: "resolved-rg-mbid", Valid: true},
|
||||
ID: id,
|
||||
},
|
||||
); err != nil {
|
||||
t.Fatalf("resolve pending release mbid: %v", err)
|
||||
}
|
||||
|
||||
pending, err := e.pendingReleaseMBIDs(db.Ctx)
|
||||
if err != nil {
|
||||
t.Fatalf("the backfill's query failed: %v", err)
|
||||
}
|
||||
|
||||
if len(pending) != 0 {
|
||||
t.Fatalf("a resolved album is still a candidate: %v", pending)
|
||||
}
|
||||
|
||||
album, err := db.ReadQueries.GetAlbum(db.Ctx, id)
|
||||
if err != nil {
|
||||
t.Fatalf("get album: %v", err)
|
||||
}
|
||||
|
||||
if album.Mbid.String != "resolved-rg-mbid" {
|
||||
t.Errorf("album mbid = %q, want the resolved one", album.Mbid.String)
|
||||
}
|
||||
|
||||
if album.PendingReleaseMbid.Valid &&
|
||||
album.PendingReleaseMbid.String != "" {
|
||||
t.Errorf(
|
||||
"the pending marker survived as %q",
|
||||
album.PendingReleaseMbid.String,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// TestAResolvedMBIDIsNeverOverwritten covers the guard in the UPDATE.
|
||||
//
|
||||
// The pass runs against rows it read earlier, and a rescan can resolve
|
||||
// an album from its tags in between -- a real MBID from the file must
|
||||
// win over one this pass inferred from a release.
|
||||
func TestAResolvedMBIDIsNeverOverwritten(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
db := database.NewTestDB(t)
|
||||
|
||||
id := seedPendingAlbum(t, db, "Pending Album", "release-mbid-1")
|
||||
|
||||
if err := db.Queries.SetAlbumMBID(db.Ctx, sqlcgen.SetAlbumMBIDParams{
|
||||
Mbid: sql.NullString{String: "from-the-tags", Valid: true},
|
||||
ID: id,
|
||||
}); err != nil {
|
||||
t.Fatalf("set album mbid: %v", err)
|
||||
}
|
||||
|
||||
if err := db.Queries.ResolveAlbumPendingReleaseMBID(
|
||||
db.Ctx,
|
||||
sqlcgen.ResolveAlbumPendingReleaseMBIDParams{
|
||||
Mbid: sql.NullString{String: "from-the-backfill", Valid: true},
|
||||
ID: id,
|
||||
},
|
||||
); err != nil {
|
||||
t.Fatalf("resolve pending release mbid: %v", err)
|
||||
}
|
||||
|
||||
album, err := db.ReadQueries.GetAlbum(db.Ctx, id)
|
||||
if err != nil {
|
||||
t.Fatalf("get album: %v", err)
|
||||
}
|
||||
|
||||
if album.Mbid.String != "from-the-tags" {
|
||||
t.Errorf(
|
||||
"album mbid = %q, want the tagged one to have won",
|
||||
album.Mbid.String,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// TestThePassIsBounded checks the LIMIT.
|
||||
//
|
||||
// Each row costs a live MusicBrainz lookup on a 1 req/s limiter shared
|
||||
// with every page the user can open, so an unbounded read is a run that
|
||||
// lasts as long as the library is untagged. The sqlc query 013 wrote
|
||||
// had no LIMIT; the raw statement it was replacing did.
|
||||
func TestThePassIsBounded(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
db := database.NewTestDB(t)
|
||||
e := newPendingTestService(db)
|
||||
|
||||
for i := range releaseGroupMBIDBackfillMaxPerRun + 10 {
|
||||
seedPendingAlbum(
|
||||
t, db,
|
||||
"Album "+string(rune('A'+i%26))+strconv.Itoa(i),
|
||||
"release-mbid-"+strconv.Itoa(i),
|
||||
)
|
||||
}
|
||||
|
||||
pending, err := e.pendingReleaseMBIDs(db.Ctx)
|
||||
if err != nil {
|
||||
t.Fatalf("the backfill's query failed: %v", err)
|
||||
}
|
||||
|
||||
if len(pending) != releaseGroupMBIDBackfillMaxPerRun {
|
||||
t.Errorf(
|
||||
"got %d albums, want the run bounded at %d",
|
||||
len(pending), releaseGroupMBIDBackfillMaxPerRun,
|
||||
)
|
||||
}
|
||||
}
|
||||
@@ -52,6 +52,75 @@ func UseHomeOverride(base string) {
|
||||
_ = os.Setenv(envHomeOverride, base)
|
||||
}
|
||||
|
||||
// envTempDir is the variable Go's os.TempDir() reads, and through it
|
||||
// every library in the process that asks for a temporary file.
|
||||
const envTempDir = "TMPDIR"
|
||||
|
||||
// tempDirName is the subdirectory of the app's own storage that
|
||||
// becomes that answer.
|
||||
const tempDirName = "tmp"
|
||||
|
||||
// UseTempDir gives the process a temporary directory that exists.
|
||||
//
|
||||
// **Android has no /tmp and hands an app no TMPDIR**, and Go's
|
||||
// os.TempDir() falls back to "/tmp" when the variable is unset -- so
|
||||
// every library in the process that wants scratch space is handed a
|
||||
// path that has never existed. SQLite is the one that noticed: an
|
||||
// INSERT ... SELECT large enough to spill returned
|
||||
// SQLITE_IOERR_GETTEMPPATH (disk I/O error 6410), which is how the
|
||||
// champion search index came to fail its rebuild on every launch while
|
||||
// the app otherwise looked healthy (#190).
|
||||
//
|
||||
// It is the *class* that is fixed here rather than that statement.
|
||||
// Anything that spills fails the same way on that platform -- large
|
||||
// sorts, large joins, VACUUM -- so the repair belongs at the process's
|
||||
// one answer to the question rather than at each caller. The
|
||||
// alternative considered was PRAGMA temp_store = MEMORY, which is
|
||||
// cheaper and more local and is a promise that every future spill fits
|
||||
// in RAM on a phone; the catalog is the largest thing in this app and
|
||||
// that is not a promise worth making silently.
|
||||
//
|
||||
// The rules are UseHomeOverride's, for the same reasons. **An empty
|
||||
// base is a no-op**, because that is what
|
||||
// application.Mobile.StoragePath() returns on desktop -- so this needs
|
||||
// no build tag and changes nothing off mobile, where /tmp is real. And
|
||||
// **an explicit TMPDIR wins**, so anyone who set one deliberately gets
|
||||
// what they asked for; nothing sets it on the platform this exists for.
|
||||
//
|
||||
// It returns its error rather than swallowing it because a temp
|
||||
// directory that could not be created is the same silent failure one
|
||||
// step earlier, and since #160 a log line on that platform is
|
||||
// something a person can actually read.
|
||||
func UseTempDir(base string) error {
|
||||
if base == "" || os.Getenv(envTempDir) != "" {
|
||||
return nil
|
||||
}
|
||||
|
||||
dir := filepath.Join(base, tempDirName)
|
||||
|
||||
if err := os.MkdirAll(dir, os.ModePerm); err != nil {
|
||||
return fmt.Errorf("could not make the temp directory %s: %w", dir, err)
|
||||
}
|
||||
|
||||
// Writability is checked rather than assumed: the whole failure
|
||||
// this repairs is a directory that is named and cannot be used, and
|
||||
// MkdirAll on an existing unwritable directory succeeds.
|
||||
probe, err := os.CreateTemp(dir, "probe")
|
||||
if err != nil {
|
||||
return fmt.Errorf("temp directory %s is not writable: %w", dir, err)
|
||||
}
|
||||
|
||||
name := probe.Name()
|
||||
_ = probe.Close()
|
||||
_ = os.Remove(name)
|
||||
|
||||
if err := os.Setenv(envTempDir, dir); err != nil {
|
||||
return fmt.Errorf("could not set %s: %w", envTempDir, err)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// getUserDirPath returns and creates the path for a user directory.
|
||||
func getUserDirPath(dt dirType) (string, error) {
|
||||
path, err := resolveUserDirPath(dt)
|
||||
|
||||
@@ -87,3 +87,116 @@ func TestUseHomeOverride(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// UseTempDir carries UseHomeOverride's two rules for the same reasons,
|
||||
// plus one of its own: the directory it names has to be usable.
|
||||
//
|
||||
// **The only tier that can compile the platform this exists for is a
|
||||
// phone**, so everything decidable off one is decided here -- which is
|
||||
// androidpayload.go's discipline, and is why the platform call is a
|
||||
// parameter rather than something this package reaches for. The
|
||||
// device's half is a single measurement: no /tmp, no TMPDIR (#190).
|
||||
func TestUseTempDir(t *testing.T) {
|
||||
t.Run("an empty base is a no-op", func(t *testing.T) {
|
||||
// This is the desktop case in full: StoragePath() answers ""
|
||||
// off mobile, where /tmp is real and must be left alone.
|
||||
t.Setenv(envTempDir, "")
|
||||
|
||||
if err := UseTempDir(""); err != nil {
|
||||
t.Fatalf("UseTempDir(\"\") = %v, want nil", err)
|
||||
}
|
||||
|
||||
if got := os.Getenv(envTempDir); got != "" {
|
||||
t.Errorf("%s = %q, want it untouched", envTempDir, got)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("an explicit TMPDIR wins", func(t *testing.T) {
|
||||
const chosen = "/somewhere/deliberate"
|
||||
|
||||
// The base is taken before TMPDIR moves, because t.TempDir()
|
||||
// reads TMPDIR too -- which is the same fact this function is
|
||||
// about, met from the other side.
|
||||
base := t.TempDir()
|
||||
|
||||
t.Setenv(envTempDir, chosen)
|
||||
|
||||
if err := UseTempDir(base); err != nil {
|
||||
t.Fatalf("UseTempDir = %v, want nil", err)
|
||||
}
|
||||
|
||||
if got := os.Getenv(envTempDir); got != chosen {
|
||||
t.Errorf("%s = %q, want the explicit %q", envTempDir, got, chosen)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("points at a real directory under the base", func(t *testing.T) {
|
||||
base := t.TempDir()
|
||||
|
||||
t.Setenv(envTempDir, "")
|
||||
|
||||
if err := UseTempDir(base); err != nil {
|
||||
t.Fatalf("UseTempDir = %v, want nil", err)
|
||||
}
|
||||
|
||||
got := os.Getenv(envTempDir)
|
||||
|
||||
want := filepath.Join(base, tempDirName)
|
||||
if got != want {
|
||||
t.Fatalf("%s = %q, want %q", envTempDir, got, want)
|
||||
}
|
||||
|
||||
// The whole failure being repaired is a temp directory that is
|
||||
// named and does not exist, so naming one is not enough.
|
||||
info, err := os.Stat(got)
|
||||
if err != nil {
|
||||
t.Fatalf("the temp directory was named but not created: %v", err)
|
||||
}
|
||||
|
||||
if !info.IsDir() {
|
||||
t.Fatalf("%s is not a directory", got)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("os.TempDir then answers with it", func(t *testing.T) {
|
||||
// The point of setting the variable at all: this is what every
|
||||
// library in the process reads, SQLite's driver included.
|
||||
base := t.TempDir()
|
||||
|
||||
t.Setenv(envTempDir, "")
|
||||
|
||||
if err := UseTempDir(base); err != nil {
|
||||
t.Fatalf("UseTempDir = %v, want nil", err)
|
||||
}
|
||||
|
||||
if got := os.TempDir(); got != filepath.Join(base, tempDirName) {
|
||||
t.Errorf("os.TempDir() = %q, want the directory we made", got)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("an unwritable directory is an error, not a silent success", func(t *testing.T) {
|
||||
if os.Getuid() == 0 {
|
||||
t.Skip("root can write anywhere, so there is nothing to refuse")
|
||||
}
|
||||
|
||||
base := t.TempDir()
|
||||
|
||||
// MkdirAll on an existing directory succeeds whatever its
|
||||
// mode, so without the write probe this case would set TMPDIR
|
||||
// to a directory nothing can use -- which is the bug again,
|
||||
// one directory over.
|
||||
if err := os.Mkdir(filepath.Join(base, tempDirName), 0o500); err != nil {
|
||||
t.Fatalf("prepare the unwritable directory: %v", err)
|
||||
}
|
||||
|
||||
t.Setenv(envTempDir, "")
|
||||
|
||||
if err := UseTempDir(base); err == nil {
|
||||
t.Fatal("UseTempDir accepted a directory it cannot write to")
|
||||
}
|
||||
|
||||
if got := os.Getenv(envTempDir); got != "" {
|
||||
t.Errorf("%s was set to %q despite the failure", envTempDir, got)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
@@ -45,18 +45,6 @@ export class SeekBar extends LitElement {
|
||||
private showRemaining: boolean = true;
|
||||
|
||||
static override styles = [designTokens, waSliderLabel, css`
|
||||
/* 12px below the phone breakpoint. The bottom bar's seek bar is
|
||||
display:none there (016 B2 phase 1), so the only instance a
|
||||
viewport media query can reach at that width is the full-screen
|
||||
now-playing view's -- which is exactly the one a thumb uses.
|
||||
The track size lives on wa-slider inside this shadow root, so a
|
||||
custom property set by the host would not reach it. */
|
||||
@media (max-width: 599px) {
|
||||
wa-slider {
|
||||
--track-size: 12px;
|
||||
}
|
||||
}
|
||||
|
||||
wa-slider {
|
||||
--track-size: 6px;
|
||||
flex: 1;
|
||||
@@ -80,6 +68,57 @@ export class SeekBar extends LitElement {
|
||||
background: var(--yj-bg-base, black);
|
||||
}
|
||||
|
||||
/* The phone's seek bar, and this block is last on purpose.
|
||||
|
||||
A media query adds no specificity, so this lived above the plain
|
||||
"wa-slider" rule and lost to it at every width: the 12px track it
|
||||
asks for had never once applied, and the bar measured 261x6 on
|
||||
the device while the source said 12. That is index.css's rule
|
||||
("the phone section is last on purpose") met inside a component's
|
||||
own stylesheet, and nothing renders differently in any tier here
|
||||
to say so.
|
||||
|
||||
The bottom bar's seek bar is display:none below this width (016
|
||||
B2 phase 1), so the only instance a viewport media query can
|
||||
reach is the full-screen now-playing view's -- which is exactly
|
||||
the one a thumb uses. The desktop bar keeps its 6px, where a
|
||||
mouse is precise and the thickness is right.
|
||||
|
||||
The painted track and the thing you can hit are allowed to
|
||||
differ, and a slider is the clearest case where they should: 12px
|
||||
is a progress bar you can see, and 44px is the app's touch floor
|
||||
(#56). A 44px-*thick* bar would be wrong-looking and would cost
|
||||
the album art the vertical space #51 spent an issue recovering.
|
||||
|
||||
Two things about how the target is built.
|
||||
|
||||
The padding goes on ::part(slider) rather than on the host,
|
||||
because that inner div is what carries the gesture -- it has the
|
||||
listener and the touch-action: none, and it is exactly the host's
|
||||
size, so padding the host would grow a box that does not take the
|
||||
press.
|
||||
|
||||
The padding is asymmetric and the margins cancel it, so the row
|
||||
does not grow by the difference. Both halves are measured: the
|
||||
seek row is 19px (its clocks, not the track, decide that) and the
|
||||
play button's top edge is 8px below it, so the target takes the
|
||||
space *above*, where .art is a non-interactive div. Growing the
|
||||
row instead cost the art 25px of 143. Verified on the device at
|
||||
424x439: hit area 44px, painted track 12px, row still 19px, art
|
||||
still 143px, 8px of clearance left under the play button, a press
|
||||
26px above the track seeks, and a hit test on the play button's
|
||||
top edge still reaches the play button. */
|
||||
@media (max-width: 599px) {
|
||||
wa-slider {
|
||||
--track-size: 12px;
|
||||
}
|
||||
|
||||
wa-slider::part(slider) {
|
||||
padding-block: 28px 4px;
|
||||
margin-block: -28px -4px;
|
||||
}
|
||||
}
|
||||
|
||||
#seek-bar-container {
|
||||
display: flex;
|
||||
justify-content: space-between;
|
||||
|
||||
@@ -298,6 +298,47 @@ export class PageHeader extends LitElement {
|
||||
flex-shrink: 0;
|
||||
}
|
||||
|
||||
/* Every control in this header meets the app's 44px touch
|
||||
floor -- the number #56 set for the transport and the
|
||||
queue header already keeps (#186).
|
||||
|
||||
It is min-size rather than padding with a negative
|
||||
margin, which is what the seek bar needed (#187), and
|
||||
the difference is worth stating because it decides
|
||||
whether targets can collide. There the painted track had
|
||||
to stay thin, so the target was grown past its own box
|
||||
and had to be checked against its neighbours. Here the
|
||||
control *is* the target: the boxes are flex items, so
|
||||
the gap keeps them apart and no two can overlap by
|
||||
construction.
|
||||
|
||||
There is no phone branch. With the target being the box,
|
||||
a 44px control on a desktop is merely large, and a
|
||||
second declaration of what a phone shows is a second
|
||||
thing to keep in step -- which is the reason this
|
||||
component has never had one. It also avoids a media
|
||||
query that no tier here renders, which is exactly how
|
||||
the seek bar's phone rule came to be dead for months.
|
||||
|
||||
**The height is the box and the width is not**, and that
|
||||
asymmetry is the whole of what the overflow fit below
|
||||
cares about. That pass measures inline size, so a taller
|
||||
control costs it nothing and a wider one costs it
|
||||
directly. Growing the two square controls to 44px wide
|
||||
added 22px, which fits at every width Chromium was
|
||||
checked at and clipped the overflow trigger at 320px in
|
||||
**WebKit** -- the engine closest to what actually ships,
|
||||
and the one no machine here can run. So the horizontal
|
||||
half is padding with the margin cancelling it, which is
|
||||
what the issue asked for in the first place: the target
|
||||
grows and the layout does not.
|
||||
|
||||
The cost is that a horizontal target can now overlap a
|
||||
neighbour, which the box version could not. The arrow's
|
||||
is deliberately lopsided for the seek bar's reason
|
||||
(#187): the select is 6px to its left and there is open
|
||||
space to its right, so it takes the side with nothing to
|
||||
steal from. */
|
||||
.sort select {
|
||||
font: inherit;
|
||||
color: inherit;
|
||||
@@ -306,6 +347,7 @@ export class PageHeader extends LitElement {
|
||||
border-radius: 4px;
|
||||
padding: 3px 6px;
|
||||
cursor: pointer;
|
||||
min-block-size: 44px;
|
||||
}
|
||||
|
||||
.sort-dir {
|
||||
@@ -318,6 +360,18 @@ export class PageHeader extends LitElement {
|
||||
color: inherit;
|
||||
cursor: pointer;
|
||||
padding: 3px 5px;
|
||||
/* 28x21 before this, the smallest control in the
|
||||
header and the only one that failed the floor in
|
||||
both directions.
|
||||
|
||||
Vertically the box grows, because the header has the
|
||||
room and nothing measures it. Horizontally the box
|
||||
must not: 28 + 2 + 14 is a 44px target over a 28px
|
||||
layout box, weighted right because the select is 6px
|
||||
to the left. */
|
||||
min-block-size: 44px;
|
||||
padding-inline: 5px 21px;
|
||||
margin-inline: 0 -16px;
|
||||
}
|
||||
|
||||
.sort-dir:hover {
|
||||
@@ -377,10 +431,20 @@ export class PageHeader extends LitElement {
|
||||
gap: 6px;
|
||||
white-space: nowrap;
|
||||
flex-shrink: 0;
|
||||
justify-content: center;
|
||||
min-block-size: 44px;
|
||||
}
|
||||
|
||||
.more-button {
|
||||
padding: 6px 10px;
|
||||
/* 38x27, and it is the route to every collapsed
|
||||
action, so it is the last control that should be
|
||||
hard to hit -- and the one WebKit clipped at 320px
|
||||
when this was 6px wider as a box. 38 + 3 + 3 is a
|
||||
44px target over a 38px layout box; the actions row
|
||||
has an 8px gap, so this one can be symmetric. */
|
||||
padding-inline: 13px;
|
||||
margin-inline: -3px;
|
||||
}
|
||||
|
||||
/* The display: flex above outranks the UA stylesheet's
|
||||
|
||||
@@ -61,11 +61,32 @@ export class SearchTrigger extends LitElement {
|
||||
display: inline-flex;
|
||||
align-items: center;
|
||||
justify-content: center;
|
||||
/* The smallest a touch target should be. The header's
|
||||
own action buttons are smaller because they carry a
|
||||
label; this one is a glyph. */
|
||||
min-width: 40px;
|
||||
min-height: 40px;
|
||||
/* The app's touch floor, from #56 -- and this is the
|
||||
control that should least have to argue for it: #57
|
||||
created it as the phone's replacement for the header
|
||||
search box, so it exists *only* where there is a
|
||||
thumb.
|
||||
|
||||
It shipped at 40px under a comment calling that "the
|
||||
smallest a touch target should be", which was the
|
||||
floor being restated four pixels short rather than a
|
||||
second opinion about it (#186). The rest of that
|
||||
comment said the header's own action buttons are
|
||||
smaller because they carry a label; they are 44px
|
||||
now too, so that no longer distinguishes anything.
|
||||
|
||||
The extra width is a target rather than a box, for
|
||||
page-header's reason: this button sits in that
|
||||
header, whose overflow fit (#69) measures inline
|
||||
size, and four pixels there is four pixels the
|
||||
trigger for every collapsed action does not get at
|
||||
320px. Height is free -- nothing measures it. */
|
||||
min-width: 44px;
|
||||
min-height: 44px;
|
||||
/* Border-box, so the 44 above is the whole target and
|
||||
the margin is what hands the four extra pixels back
|
||||
to the row. */
|
||||
margin-inline: -2px;
|
||||
padding: 0;
|
||||
background: none;
|
||||
border: 1px solid var(--yj-border-subtle, #555);
|
||||
|
||||
@@ -99,6 +99,25 @@ describe('<search-trigger>', () => {
|
||||
}
|
||||
});
|
||||
|
||||
it('meets the touch floor it was shipped four pixels under', async () => {
|
||||
stubPhone(true);
|
||||
|
||||
// #57 created this as the phone's replacement for the header search
|
||||
// box, so it exists *only* where there is a thumb -- and it shipped
|
||||
// at 40x40 under a comment calling that "the smallest a touch
|
||||
// target should be", which was the app's own 44px floor (#56)
|
||||
// restated short rather than a second opinion about it. #186.
|
||||
const el = await fixture('search-trigger');
|
||||
const button = shadow<HTMLButtonElement>(el, '[data-testid="search-trigger"]');
|
||||
|
||||
expect(button).not.toBeNull();
|
||||
|
||||
const box = button!.getBoundingClientRect();
|
||||
|
||||
expect(Math.round(box.width)).toBeGreaterThanOrEqual(44);
|
||||
expect(Math.round(box.height)).toBeGreaterThanOrEqual(44);
|
||||
});
|
||||
|
||||
it('names what the button will search', async () => {
|
||||
stubPhone(true);
|
||||
|
||||
|
||||
@@ -0,0 +1,211 @@
|
||||
/**
|
||||
* The seek bar's painted track and the thing you can hit are allowed to
|
||||
* differ, and a slider is the clearest case where they should.
|
||||
*
|
||||
* On `now-playing-view` — the screen that exists so a phone has
|
||||
* somewhere to seek from — the slider measured 261x6 on the reference
|
||||
* device (#187). Six pixels is the whole of the drag target on the
|
||||
* app's primary seeking affordance, against a 44px floor the app set
|
||||
* for itself in #56 and holds to in the queue panel.
|
||||
*
|
||||
* Two separate faults, and the first is why the second was not obvious.
|
||||
*
|
||||
* **The phone rule had never applied.** `seek-bar`'s stylesheet asked
|
||||
* for a 12px track below 599px and then set 6px in a plain `wa-slider`
|
||||
* rule *written after it*. A media query adds no specificity, so the
|
||||
* plain rule won at every width — which is `index.css`'s documented
|
||||
* rule ("the phone section is last on purpose") reproduced inside a
|
||||
* component's own stylesheet. The source said 12 and the device said 6.
|
||||
*
|
||||
* **And 12px would still be under the floor**, so the target is built
|
||||
* around the track rather than by thickening it: padding on the part
|
||||
* that carries the gesture, with margins cancelling it so the row does
|
||||
* not grow.
|
||||
*
|
||||
* This is asserted against the *parsed stylesheet*, on
|
||||
* `hover-affordance.test.ts`'s precedent and with the same limitation
|
||||
* stated rather than hidden: no tier here renders at a phone width with
|
||||
* a real `wa-slider` laid out, so what can be checked is the shape the
|
||||
* browser built from the css`` literal. The pixel measurements that
|
||||
* chose these numbers were taken on the device and are recorded on
|
||||
* #187 and in the stylesheet's own comment — a number measured on a
|
||||
* phone is not a number CI can assert.
|
||||
*
|
||||
* Which is the regression worth catching anyway. Both failures are
|
||||
* invisible on a desktop: hoisting the block back above the plain rule
|
||||
* renders identically at every width CI runs at, and it is exactly what
|
||||
* a tidy-up does.
|
||||
*/
|
||||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
import '@components/audio-player/seekbar/seek-bar';
|
||||
import { fixture } from '@test/support/render';
|
||||
|
||||
/** The app's touch floor, from #56. */
|
||||
const TOUCH_FLOOR = 44;
|
||||
|
||||
/** The width below which the phone's rules apply. */
|
||||
const PHONE_QUERY = /max-width:\s*599px/;
|
||||
|
||||
type Rule = { text: string; condition: string | null };
|
||||
|
||||
/**
|
||||
* Every rule in the element's own adopted stylesheets, flattened **in
|
||||
* order**, which is the whole point here: the fault being guarded is a
|
||||
* rule sitting in the wrong place, not a rule being absent.
|
||||
*/
|
||||
function rulesOf(host: Element): Rule[] {
|
||||
const sheets = host.shadowRoot?.adoptedStyleSheets ?? [];
|
||||
const out: Rule[] = [];
|
||||
|
||||
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;
|
||||
}
|
||||
|
||||
/**
|
||||
* The two px numbers of a `*-block` declaration, as [start, end].
|
||||
*
|
||||
* A symmetric pair is **serialised back as one value** — `padding-block:
|
||||
* 16px 16px` reads as `padding-block: 16px` — so a naive pair-reader
|
||||
* fails on the shorthand rather than on the thing it is checking, and
|
||||
* says the wrong thing about why. That is not hypothetical: it is what
|
||||
* the symmetric-padding reversion did while this test was being
|
||||
* proved.
|
||||
*/
|
||||
function blockPair(text: string, property: string): [number, number] | null {
|
||||
const declaration = new RegExp(`${property}:\\s*([^;]+)`).exec(text)?.[1];
|
||||
|
||||
if (declaration === undefined) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const values = [...declaration.matchAll(/(-?[\d.]+)px/g)].map((m) =>
|
||||
Number(m[1]),
|
||||
);
|
||||
|
||||
const [start, end] = values;
|
||||
|
||||
if (start === undefined) {
|
||||
return null;
|
||||
}
|
||||
|
||||
return [start, end ?? start];
|
||||
}
|
||||
|
||||
describe("the seek bar's phone rules", () => {
|
||||
it('are last, so they are not silently overridden', async () => {
|
||||
const el = await fixture('seek-bar', {});
|
||||
const rules = rulesOf(el);
|
||||
|
||||
// A sweep that read nothing passes vacuously — the same first
|
||||
// assertion icon-language.test.ts makes, for the same reason.
|
||||
expect(rules.length).toBeGreaterThan(0);
|
||||
|
||||
const declaresTrackSize = (r: Rule) => /--track-size:/.test(r.text);
|
||||
|
||||
const lastUnconditional = rules.findLastIndex(
|
||||
(r) => r.condition === null && declaresTrackSize(r),
|
||||
);
|
||||
const phoneOverride = rules.findLastIndex(
|
||||
(r) => r.condition !== null && PHONE_QUERY.test(r.condition)
|
||||
&& declaresTrackSize(r),
|
||||
);
|
||||
|
||||
expect(lastUnconditional).toBeGreaterThanOrEqual(0);
|
||||
expect(phoneOverride).toBeGreaterThanOrEqual(0);
|
||||
|
||||
// A media query adds no specificity. Written first, it loses.
|
||||
expect(phoneOverride).toBeGreaterThan(lastUnconditional);
|
||||
});
|
||||
|
||||
it('give the slider a pointer target of at least the touch floor', async () => {
|
||||
const el = await fixture('seek-bar', {});
|
||||
const rules = rulesOf(el);
|
||||
|
||||
const track = rules.find(
|
||||
(r) => r.condition !== null && PHONE_QUERY.test(r.condition)
|
||||
&& /--track-size:/.test(r.text),
|
||||
);
|
||||
const target = rules.find(
|
||||
(r) => r.condition !== null && PHONE_QUERY.test(r.condition)
|
||||
&& r.text.includes('::part(slider)'),
|
||||
);
|
||||
|
||||
expect(track).toBeDefined();
|
||||
expect(target).toBeDefined();
|
||||
|
||||
const trackSize = Number(
|
||||
/--track-size:\s*(-?[\d.]+)px/.exec(track!.text)?.[1],
|
||||
);
|
||||
const padding = blockPair(target!.text, 'padding-block');
|
||||
|
||||
expect(padding).not.toBeNull();
|
||||
|
||||
// The padding is on ::part(slider) rather than on the host because
|
||||
// that inner div is what carries the gesture: it has the listener
|
||||
// and the touch-action, and it is exactly the host's size, so
|
||||
// padding the host grows a box that does not take the press.
|
||||
const hitArea = trackSize + padding![0] + padding![1];
|
||||
|
||||
expect(hitArea).toBeGreaterThanOrEqual(TOUCH_FLOOR);
|
||||
});
|
||||
|
||||
it('do not grow the row they sit in', async () => {
|
||||
const el = await fixture('seek-bar', {});
|
||||
|
||||
const target = rulesOf(el).find(
|
||||
(r) => r.condition !== null && PHONE_QUERY.test(r.condition)
|
||||
&& r.text.includes('::part(slider)'),
|
||||
);
|
||||
|
||||
expect(target).toBeDefined();
|
||||
|
||||
const padding = blockPair(target!.text, 'padding-block');
|
||||
const margin = blockPair(target!.text, 'margin-block');
|
||||
|
||||
expect(padding).not.toBeNull();
|
||||
expect(margin).not.toBeNull();
|
||||
|
||||
// now-playing-view's vertical budget is fixed and #51 measured
|
||||
// every pixel of it: letting the row grow by the difference cost
|
||||
// the album art 25px of 143 when it was tried on the device.
|
||||
expect(margin![0]).toBe(-padding![0]);
|
||||
expect(margin![1]).toBe(-padding![1]);
|
||||
});
|
||||
|
||||
it('take the space above, because what is below is the transport', async () => {
|
||||
const el = await fixture('seek-bar', {});
|
||||
|
||||
const target = rulesOf(el).find(
|
||||
(r) => r.condition !== null && PHONE_QUERY.test(r.condition)
|
||||
&& r.text.includes('::part(slider)'),
|
||||
);
|
||||
|
||||
expect(target).toBeDefined();
|
||||
|
||||
const pair = blockPair(target!.text, 'padding-block');
|
||||
|
||||
expect(pair).not.toBeNull();
|
||||
|
||||
const [above, below] = pair!;
|
||||
|
||||
// Measured at 424x439: the seek row is 19px and the play button's
|
||||
// top edge is 8px below it, while `.art` above is a non-interactive
|
||||
// div. A symmetric target would reach into the play button — the
|
||||
// most important control on the screen — so the growth is upward.
|
||||
expect(above).toBeGreaterThan(below);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,162 @@
|
||||
/**
|
||||
* Every control a finger meets is at least 44px (#186).
|
||||
*
|
||||
* #56 sized the playback transport for a thumb and named 44px; the
|
||||
* queue header keeps it; nothing else was resized. So the controls a
|
||||
* user meets on *every* screen — the sort control, its direction
|
||||
* button, the page actions, the overflow trigger and the phone's search
|
||||
* button — sat between a third and two thirds of the app's own floor.
|
||||
* Measured on the reference device (TLP301, 424x439): `page-sort` 99x23,
|
||||
* `page-sort-direction` **28x21**, `page-actions-more` 38x27,
|
||||
* `search-trigger` 40x40.
|
||||
*
|
||||
* Unlike the seek bar's target (#187), this one can be measured here
|
||||
* rather than inferred from the stylesheet. There the painted track had
|
||||
* to stay thin, so the hit area was grown past its own box and only a
|
||||
* phone-width layout of a third-party slider could show it. Here the
|
||||
* control *is* the target, so a real Chromium rendering a real
|
||||
* `page-header` gives the actual answer — and because it is a `min-size`
|
||||
* rather than a media query, the answer is the same at every width,
|
||||
* which is what makes it checkable in this tier at all.
|
||||
*
|
||||
* That is also why there is no phone branch to test: a 44px control on
|
||||
* a desktop is merely large, and a second declaration of what a phone
|
||||
* shows is a second thing to keep in step.
|
||||
*/
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import type { PageAction, PageHeader } from '@components/page-header/page-header';
|
||||
|
||||
import '@components/page-header/page-header';
|
||||
import { fixture, shadowAll } from '@test/support/render';
|
||||
|
||||
/** The app's touch floor, from #56. */
|
||||
const FLOOR = 44;
|
||||
|
||||
const SORTS = [
|
||||
{ id: 'name', label: 'Name' },
|
||||
{ id: 'tracks', label: 'Tracks' },
|
||||
];
|
||||
|
||||
function actions(): PageAction[] {
|
||||
return [
|
||||
{ id: 'import', label: 'Import', icon: 'file-import', priority: 0, onSelect: () => {} },
|
||||
{ id: 'new', label: 'New Playlist', icon: 'plus', priority: 2, onSelect: () => {} },
|
||||
];
|
||||
}
|
||||
|
||||
/** Every visible control in the header's own shadow root. */
|
||||
function controlsOf(el: PageHeader): { name: string; el: HTMLElement }[] {
|
||||
return shadowAll<HTMLElement>(el, 'button, select')
|
||||
.filter((c) => !(c as HTMLButtonElement).hidden)
|
||||
.map((c) => ({
|
||||
name: c.dataset.testid ?? (c.className || c.tagName.toLowerCase()),
|
||||
el: c,
|
||||
}));
|
||||
}
|
||||
|
||||
function tooSmall(controls: { name: string; el: HTMLElement }[]): string[] {
|
||||
return controls
|
||||
.map(({ name, el }) => {
|
||||
const b = el.getBoundingClientRect();
|
||||
|
||||
return { name, w: Math.round(b.width), h: Math.round(b.height) };
|
||||
})
|
||||
.filter((c) => c.w < FLOOR || c.h < FLOOR)
|
||||
.map((c) => `${c.name} ${c.w}x${c.h}`);
|
||||
}
|
||||
|
||||
describe("the page header's controls", () => {
|
||||
it('all meet the touch floor', async () => {
|
||||
const el = await fixture<PageHeader>('page-header', {
|
||||
heading: 'Playlists',
|
||||
count: 50,
|
||||
countNoun: 'playlist',
|
||||
sortOptions: SORTS,
|
||||
sortField: 'name',
|
||||
sortDirection: 'asc',
|
||||
actions: actions(),
|
||||
});
|
||||
|
||||
const controls = controlsOf(el);
|
||||
|
||||
// A sweep that found no controls passes vacuously — the same first
|
||||
// assertion icon-language.test.ts makes, for the same reason.
|
||||
expect(controls.length).toBeGreaterThan(0);
|
||||
|
||||
// The two that were smallest, named so a regression says which.
|
||||
expect(controls.map((c) => c.name)).toContain('page-sort-direction');
|
||||
expect(controls.map((c) => c.name)).toContain('page-sort');
|
||||
|
||||
expect(tooSmall(controls)).toEqual([]);
|
||||
});
|
||||
|
||||
it('grows the target without growing the box, so the overflow fit is untouched', async () => {
|
||||
// The regression this exists for, and it was a real one: growing
|
||||
// the two square controls to 44px *wide* added 22px to the header,
|
||||
// which fit at every width Chromium was checked at and clipped the
|
||||
// overflow trigger at 320x600 in WebKit -- the engine closest to
|
||||
// what ships, and the one no machine here can run. #69's fit pass
|
||||
// measures inline size, so a taller control is free and a wider one
|
||||
// is not.
|
||||
//
|
||||
// Negative inline margins are what keep the box out of it: the
|
||||
// padding makes the target, and the margin gives the space back.
|
||||
const el = await fixture<PageHeader>('page-header', {
|
||||
heading: 'Playlists',
|
||||
sortOptions: SORTS,
|
||||
sortField: 'name',
|
||||
actions: actions(),
|
||||
});
|
||||
|
||||
el.style.width = '320px';
|
||||
|
||||
for (let frame = 0; frame < 3; frame += 1) {
|
||||
await new Promise((r) => requestAnimationFrame(r));
|
||||
await el.updateComplete;
|
||||
}
|
||||
|
||||
for (const selector of ['.sort-dir', '.more-button']) {
|
||||
const control = shadowAll<HTMLElement>(el, selector).filter(
|
||||
(c) => !(c as HTMLButtonElement).hidden,
|
||||
)[0];
|
||||
|
||||
expect(control, selector).toBeTruthy();
|
||||
|
||||
const style = getComputedStyle(control!);
|
||||
const added =
|
||||
parseFloat(style.marginInlineStart) + parseFloat(style.marginInlineEnd);
|
||||
|
||||
expect(added, `${selector} gives its extra width back`).toBeLessThan(0);
|
||||
}
|
||||
});
|
||||
|
||||
it('includes the overflow trigger, which is the route to the rest', async () => {
|
||||
// At 320px the fit pass collapses actions into the menu, so the
|
||||
// trigger is rendered — and it is then the only way to reach them,
|
||||
// which makes it the last control that should be hard to hit.
|
||||
const el = await fixture<PageHeader>('page-header', {
|
||||
heading: 'Playlists',
|
||||
sortOptions: SORTS,
|
||||
sortField: 'name',
|
||||
actions: actions(),
|
||||
});
|
||||
|
||||
el.style.width = '320px';
|
||||
|
||||
for (let frame = 0; frame < 3; frame += 1) {
|
||||
await new Promise((r) => requestAnimationFrame(r));
|
||||
await el.updateComplete;
|
||||
}
|
||||
|
||||
const more = shadowAll<HTMLButtonElement>(el, '.more-button').filter(
|
||||
(b) => !b.hidden,
|
||||
);
|
||||
|
||||
expect(more.length).toBe(1);
|
||||
|
||||
const box = more[0]!.getBoundingClientRect();
|
||||
|
||||
expect(Math.round(box.width)).toBeGreaterThanOrEqual(FLOOR);
|
||||
expect(Math.round(box.height)).toBeGreaterThanOrEqual(FLOOR);
|
||||
});
|
||||
});
|
||||
@@ -113,6 +113,19 @@ func main() {
|
||||
slog.SetDefault(sLogger)
|
||||
sLogger.Info("starting yellowjacket", "version", version, "commit", commit)
|
||||
|
||||
// Android has no /tmp and gives an app no TMPDIR, so anything in
|
||||
// this process that spills to a temporary file is handed a path that
|
||||
// does not exist -- see system.UseTempDir. It runs here rather than
|
||||
// beside UseHomeOverride above because it has something to say when
|
||||
// it fails and the logger does not exist up there; what matters is
|
||||
// that it is before NewYellowJacketApp, which opens the database.
|
||||
//
|
||||
// A failure is not fatal: it leaves the platform's own answer in
|
||||
// place, which is what every release before this one ran with.
|
||||
if err := system.UseTempDir(application.Mobile.StoragePath()); err != nil {
|
||||
sLogger.Error("could not set up a temp directory", "err", err.Error())
|
||||
}
|
||||
|
||||
// Start profiling server (pprof + trace). In production builds this
|
||||
// is a no-op — the compiler eliminates all profiling code.
|
||||
stopProfiler := profiling.Start(sLogger)
|
||||
|
||||
Reference in New Issue
Block a user