Skip to content

Commit 10883eb

Browse files
committed
fix: Better handling of 0 duration tracks
* Check for zero duration before using progress bar in debug logging * Adds tests for scrobble thresholds to make sure 0 duration is handled
1 parent e89ebe7 commit 10883eb

3 files changed

Lines changed: 55 additions & 3 deletions

File tree

src/backend/sources/PlayerState/AbstractPlayerState.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -351,11 +351,11 @@ export abstract class AbstractPlayerState {
351351
}
352352
parts.push(`Reported: ${this.reportedStatus.toUpperCase()} | Calculated: ${this.calculatedStatus.toUpperCase()} | Stale: ${this.isUpdateStale() ? 'Yes' : 'No'} | Orphaned: ${this.isOrphaned() ? 'Yes' : 'No'} | Player Updated At: ${todayAwareFormat(this.stateLastUpdatedAt)} | Play Updated At: ${this.playLastUpdatedAt === undefined ? 'N/A' : todayAwareFormat(this.playLastUpdatedAt)}`);
353353
let progress = '';
354-
if (this.currentListenRange !== undefined && this.currentListenRange instanceof ListenRangePositional && this.currentPlay.data.duration !== undefined) {
354+
if (this.currentListenRange !== undefined && this.currentListenRange instanceof ListenRangePositional && this.currentPlay.data.duration !== undefined && this.currentPlay.data.duration !== 0) {
355355
progress = `${progressBar(this.currentListenRange.end.position / this.currentPlay.data.duration, 1, 15)} ${formatNumber(this.currentListenRange.end.position, {toFixed: 0})}/${formatNumber(this.currentPlay.data.duration, {toFixed: 0})}s | `;
356356
}
357357
let listenedPercent = '';
358-
if (this.currentPlay !== undefined && this.currentPlay.data.duration !== undefined) {
358+
if (this.currentPlay !== undefined && this.currentPlay.data.duration !== undefined && this.currentPlay.data.duration !== 0) {
359359
listenedPercent = formatNumber((this.getListenDuration() / this.currentPlay.data.duration) * 100, {
360360
suffix: '%',
361361
toFixed: 0

src/backend/tests/player/player.test.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ describe('Player listen ranges', function () {
207207

208208
describe('When source does provide playback position', function () {
209209

210-
it('Duration is position based', function () {
210+
it('Listened duration is position based', function () {
211211
const player = new TestPositionalPlayerState(logger, [NO_DEVICE, NO_USER]);
212212

213213
const positioned = clone(newPlay);
@@ -220,6 +220,20 @@ describe('Player listen ranges', function () {
220220
assert.equal(player.getListenDuration(), 7);
221221
});
222222

223+
it('Listened duration is track duration invariant', function () {
224+
const player = new TestPositionalPlayerState(logger, [NO_DEVICE, NO_USER]);
225+
226+
const positioned = clone(newPlay);
227+
positioned.data.duration = 0;
228+
229+
player.update(testState({play: positioned, position: 3}));
230+
231+
player.currentListenRange.rtPlayer.setPosition(10000);
232+
player.update(testState({play: positioned, position: 10}), dayjs().add(10, 'seconds'));
233+
234+
assert.equal(player.getListenDuration(), 7);
235+
});
236+
223237
it('Range ends if position over drifts', function () {
224238
const player = new TestPositionalPlayerState(logger, [NO_DEVICE, NO_USER]);
225239

src/backend/tests/source/source.test.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import { generatePlay } from "../utils/PlayTestUtils.js";
1010
import { TestSource } from "./TestSource.js";
1111
import spotifyPayload from '../plays/spotifyCurrentPlaybackState.json';
1212
import SpotifySource from "../../sources/SpotifySource.js";
13+
import { timePassesScrobbleThreshold } from "../../utils/TimeUtils.js";
14+
import { DEFAULT_SCROBBLE_DURATION_THRESHOLD, DEFAULT_SCROBBLE_PERCENT_THRESHOLD } from "../../common/infrastructure/Atomic.js";
1315

1416
chai.use(asPromised);
1517

@@ -159,4 +161,40 @@ describe('Sources correctly parse incoming payloads', function () {
159161
expect(identicalArtistsPlay.data.artists).eql(['Dubmood', 'MASTER BOOT RECORD']);
160162
expect(identicalArtistsPlay.data.albumArtists).to.be.empty;
161163
});
164+
});
165+
166+
describe('Scrobble Threshold Checks', function() {
167+
168+
it('uses defaults when no user-configured thresholds are passed', function() {
169+
const results = timePassesScrobbleThreshold({}, 1, 1);
170+
expect(results.duration.threshold).to.eq(DEFAULT_SCROBBLE_DURATION_THRESHOLD);
171+
expect(results.percent.threshold).to.eq(DEFAULT_SCROBBLE_PERCENT_THRESHOLD);
172+
});
173+
174+
it('uses user-configured thresholds when passed', function() {
175+
const results = timePassesScrobbleThreshold({
176+
duration: 20,
177+
percent: 15
178+
}, 1, 1);
179+
expect(results.duration.threshold).to.eq(20);
180+
expect(results.percent.threshold).to.eq(15);
181+
});
182+
183+
it('passes when duration is above threshold', function() {
184+
const results = timePassesScrobbleThreshold({}, DEFAULT_SCROBBLE_DURATION_THRESHOLD + 1);
185+
expect(results.duration.passes).is.true;
186+
expect(results.passes).is.true;
187+
});
188+
189+
it('passes when percent is above threshold', function() {
190+
const results = timePassesScrobbleThreshold({}, 30, 50);
191+
expect(results.percent.passes).is.true;
192+
expect(results.passes).is.true;
193+
});
194+
195+
it('handles zero duration', function() {
196+
const results = timePassesScrobbleThreshold({}, DEFAULT_SCROBBLE_DURATION_THRESHOLD + 1, 0);
197+
expect(results.duration.passes).is.true;
198+
expect(results.passes).is.true;
199+
});
162200
});

0 commit comments

Comments
 (0)