Skip to content

Comments

Move VideoConfigService tests near the service#38008

Merged
farhan merged 1 commit intomasterfrom
farhan/move-video-service-test
Feb 24, 2026
Merged

Move VideoConfigService tests near the service#38008
farhan merged 1 commit intomasterfrom
farhan/move-video-service-test

Conversation

@farhan
Copy link
Contributor

@farhan farhan commented Feb 13, 2026

Move VideoConfigService tests near the app

In the test cases we are migrating we are actually testing the functionality of VideoConfigService.

It felt required while shifting test_video.py test case from edx-platform to xblocks-contrib
openedx/xblocks-contrib#144

@farhan farhan force-pushed the farhan/move-video-service-test branch from 5ff3b1f to 2472bbe Compare February 13, 2026 16:18
@farhan farhan changed the title Move VideoConfigService tests near inside its app Move VideoConfigService tests near the service Feb 21, 2026
@farhan farhan marked this pull request as ready for review February 21, 2026 06:53
@farhan farhan moved this to 👀 In review in Aximprovements Team Feb 23, 2026
@farhan farhan self-assigned this Feb 23, 2026
@salman2013
Copy link
Contributor

@farhan Just a question for more context, as per my understanding these tests are for testing videos with multiple or no transcripts translation retrieval. Why we need to move them into new class test_service?

@farhan
Copy link
Contributor Author

farhan commented Feb 23, 2026

@farhan Just a question for more context, as per my understanding these tests are for testing videos with multiple or no transcripts translation retrieval. Why we need to move them into new class test_service?

Actually, these are very old tests written before VideoConfigService existed.
We are actually testing the available_translations method which has been moved to VideoConfigService here

@salman2013
Copy link
Contributor

salman2013 commented Feb 23, 2026

@farhan Thanks for the elaborating, why we are making a separate class to move these tests? As tests are already using service to reach these methods.

@farhan
Copy link
Contributor Author

farhan commented Feb 23, 2026

@farhan Thanks for the elaborating, why we are making a separate class to move these tests? As tests are already using service to reach these methods.

We are moving test_video.py test to xblocks-contrib, PR shared in the description. Tests belong to VideoConfigService are not suppose to move into the xblocks-contrib. We will eventually remove test_video.py from the edx-platform

@farhan farhan force-pushed the farhan/move-video-service-test branch from 2472bbe to 257a387 Compare February 24, 2026 06:08
Copy link
Contributor

@salman2013 salman2013 left a comment

Choose a reason for hiding this comment

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

LGTM please merge once tests are completed.

@farhan farhan merged commit ee35515 into master Feb 24, 2026
69 checks passed
@farhan farhan deleted the farhan/move-video-service-test branch February 24, 2026 07:18
@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in Aximprovements Team Feb 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

2 participants