Skip to content

Conversation

@codomposer
Copy link

@codomposer codomposer commented Nov 10, 2025

Add Unit Tests for Missing Controllers

Summary

This PR adds comprehensive unit tests for three controllers that were previously missing test coverage:

  • GapController (streaming/controllers)
  • CatchupController (streaming/controllers)
  • ContentSteeringController (dash/controllers)

Changes Made

New Test Files

  1. test/unit/test/streaming/streaming.controllers.GapController.js

    • Tests for initialization and configuration
    • Tests for event handling (WALLCLOCK_TIME_UPDATED, PLAYBACK_SEEKING, etc.)
    • Tests for gap detection logic under various conditions
    • Tests for configuration settings (jumpGaps, jumpLargeGaps, etc.)
  2. test/unit/test/streaming/streaming.controllers.CatchupController.js

    • Tests for initialization and configuration
    • Tests for reset functionality
    • Tests for event handling (BUFFER_LEVEL_UPDATED, PLAYBACK_PROGRESS, etc.)
    • Tests for catchup mode behavior (default and LoL+)
    • Tests for buffer state management and playback rate adjustments
  3. test/unit/test/dash/dash.controllers.ContentSteeringController.js

    • Tests for initialization and configuration
    • Tests for reset functionality
    • Tests for retrieving steering data from manifest
    • Tests for handling steering responses
    • Tests for event handling (FRAGMENT_LOADING_STARTED, THROUGHPUT_MEASUREMENT_STORED, etc.)
    • Tests for synthesizing BaseURL and Location elements

Mock Updates

Enhanced existing mocks to support the new tests:

  1. test/unit/mocks/MediaPlayerModelMock.js

    • Added getCatchupModeEnabled() method
    • Added getCatchupMaxDrift() method
  2. test/unit/mocks/PlaybackControllerMock.js

    • Added seekToCurrentLive() method
    • Added getBufferLevel() method
    • Added getPlaybackStalled() method
    • Added getStreamEndTime() method
  3. test/unit/mocks/StreamControllerMock.js

    • Added getIsStreamSwitchInProgress() method
    • Added getHasMediaOrInitialisationError() method
    • Added getStreamForTime() method

Test Coverage

All tests follow the existing project patterns:

  • Use Mocha as the test framework
  • Use Chai for assertions
  • Use Sinon for spies and stubs
  • Properly mock dependencies using existing mock objects
  • Test both positive and negative scenarios
  • Cover edge cases and error conditions

Closes #4880

Contribution by Gittensor, learn more at https://gittensor.io/

@codomposer
Copy link
Author

@dsilhavy
I've added unit tests for a few controllers that were missing test coverage, can you please check this pull request?

@dsilhavy dsilhavy self-requested a review November 11, 2025 08:31
@dsilhavy dsilhavy added this to the 5.1.1 milestone Nov 11, 2025
@dsilhavy dsilhavy moved this to In Progress in dash.js Version 5.1.1 Nov 17, 2025
@codomposer
Copy link
Author

@dsilhavy
have you had a chance to check this pr?

@dsilhavy
Copy link
Collaborator

@dsilhavy have you had a chance to check this pr?

@codomposer No not yet, I have assigned it to the v5.1.1 which is targeted for end of December. We will check it within this development cycle once 5.1.0 is out.

Copy link
Collaborator

@dsilhavy dsilhavy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the pull request @codomposer . I added some comments.

In general, it looks to me like the majority of these new testcases are AI generated. While it is perfectly reasonable to use AI for this (I do it too ofc) I think some testcases need clarification and refinement. Especially when it comes to checking if the triggered events are actually leading to the desired behavior. This might only be possible to a certain degree as most of the events trigger a private function of the corresponding class. However, most testcases are not even using expect so I assume the goal was to check if any error is thrown?

@codomposer codomposer requested a review from dsilhavy November 26, 2025 16:56
@codomposer codomposer closed this Dec 2, 2025
@github-project-automation github-project-automation bot moved this from In Progress to Done in dash.js Version 5.1.1 Dec 2, 2025
@dsilhavy
Copy link
Collaborator

dsilhavy commented Dec 2, 2025

@codomposer Why did you close the PR?

@codomposer codomposer reopened this Dec 2, 2025
@codomposer
Copy link
Author

@codomposer Why did you close the PR?

Sorry, closed by mistake

@dsilhavy
Copy link
Collaborator

dsilhavy commented Dec 2, 2025

@codomposer Why did you close the PR?

Sorry, closed by mistake

Np, thanks for reopening. I will look at the PR this week

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

2 participants