Reorganize tests by Kicer86 · Pull Request #121 · Kicer86/TwoTone

Kicer86 · 2026-03-09T14:26:14Z

No description provided.

Add conftest.py that ensures tests/ is on sys.path so test modules in subdirectories can import from common. Add __init__.py files to all tool-specific test subdirectories.

Relocate all merge-related test files under tests/merge/: - test_merge_subtitles.py → merge/test_subtitles.py - test_merge_language_detection.py → merge/test_language_detection.py - test_merge_errors_reaction.py → merge/test_errors_reaction.py - test_merge_subtitles_conversion.py → merge/test_subtitles_conversion.py - test_mkvmerge_subtitle_formats.py → merge/test_mkvmerge_subtitle_formats.py

Relocate tool-specific test files: - test_concatenate.py → concatenate/test_concatenate.py - test_language_fixer.py → language_fixer/test_language_fixer.py - test_subtitles_fixer.py → subtitles_fixer/test_subtitles_fixer.py - test_transcoder.py → transcode/test_transcoder.py - test_utilities_scenes.py → utilities/test_scenes.py

Relocate utility test files: - test_utils.py → utils/test_video_utils.py - test_requirements_utils.py → utils/test_requirements_utils.py

Break the monolithic 3337-line test_melt.py into 7 focused files under tests/melt/: - helpers.py: shared MeltTestBase class, setUp fixtures, and module-level utilities (normalize, all_key_orders, etc.) - test_integration.py: MeltIntegrationTest — basic duplicate detection, dry-run, inputs, length mismatch, multi-input, stream ordering, subtitles, attachments (16 tests) - test_streams_picker.py: StreamsPickerTest — audio/video preference, force-all-streams, argument parsing (14 tests) - test_pair_matcher_integration.py: PairMatcherIntegrationTest — real video pair matching, edge cases, coverage summary (15 tests) - test_audio_alignment.py: AudioAlignmentTest — beep-based audio timing verification after melt (1 test) - test_performer_unit.py: MeltPerformerUnitTest — audio patching, stream sorting, FLAC concat, mkvmerge args, sync offset (41 tests) - test_pair_matcher_unit.py: PairMatcherUnitTest — extrapolation, edge snapping, constant-offset detection, boundary search (25 tests) All 111 melt tests pass.

The test_force_all_streams_parser_requires_preceding_input test correctly asserted SystemExit, but argparse.error() prints usage and error message to stderr before raising. Redirect stderr with contextlib.redirect_stderr to keep test output clean.

Relocate audio/, images/, videos/ (submodules), subtitles/ and subtitles_txt/ into tests/media/ to clearly separate test fixture data from test code. Replace hardcoded current_path/videos/ paths in test_subtitles_fixer with get_video() helper. Add media_path variable in common.py and update all fixture accessor functions to use it.

This reverts commit 105841d.

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Reorganizes the test suite by splitting the previously monolithic melt test module into a structured tests/melt/ package, and updates test media path handling to align with the new tests/media/ submodule layout.

Changes:

Split tests/test_melt.py into multiple focused test modules under tests/melt/ with shared utilities in tests/melt/helpers.py.
Migrate test media references to tests/media/* (via .gitmodules + tests/common.py) and update subtitles fixer tests to use get_video().
Add tests/conftest.py to make from common import ... work from subdirectories and harden run_twotone() against mutable default args.

Reviewed changes

Copilot reviewed 19 out of 48 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tests/utils/init.py	Adds package marker for `tests/utils` (currently includes whitespace-only content).
tests/utilities/init.py	Adds package marker for `tests/utilities` (currently includes whitespace-only content).
tests/transcode/init.py	Adds package marker for `tests/transcode` (currently includes whitespace-only content).
tests/test_subtitles_fixer.py	Switches video fixture path usage from `current_path` to `get_video()`.
tests/test_melt.py	Removes the large combined melt test module in favor of per-area modules.
tests/subtitles_fixer/init.py	Adds package marker for `tests/subtitles_fixer` (currently includes whitespace-only content).
tests/merge/init.py	Adds package marker for `tests/merge` (currently includes whitespace-only content).
tests/melt/test_streams_picker.py	New streams-picker focused tests + shared helper imports.
tests/melt/test_performer_unit.py	New MeltPerformer unit tests module extracted from prior suite.
tests/melt/test_pair_matcher_unit.py	New PairMatcher unit tests module extracted from prior suite.
tests/melt/test_pair_matcher_integration.py	New PairMatcher integration tests module extracted from prior suite.
tests/melt/test_integration.py	New melt end-to-end integration tests module extracted from prior suite.
tests/melt/test_audio_alignment.py	New audio-alignment integration test module extracted from prior suite.
tests/melt/helpers.py	Shared helper functions + `MeltTestBase` fixture generation extracted from prior suite.
tests/melt/init.py	Adds package marker for `tests/melt` (currently includes whitespace-only content).
tests/language_fixer/init.py	Adds package marker for `tests/language_fixer` (currently includes whitespace-only content).
tests/conftest.py	Adds `tests/` to `sys.path` to support `from common import ...` across subpackages.
tests/concatenate/init.py	Adds package marker for `tests/concatenate` (currently includes whitespace-only content).
tests/common.py	Introduces `media_path` and updates media/subtitle/audio/image path helpers accordingly; fixes `run_twotone()` defaults.
.gitmodules	Moves media submodules to `tests/media/{videos,audio,images}`.

Comments suppressed due to low confidence (2)

tests/melt/test_streams_picker.py:1

TwoToneTestCase is imported but not referenced anywhere in this file. Please remove the unused import to avoid lint/test hygiene failures (e.g., flake8/ruff F401).
tests/melt/test_streams_picker.py:1
MeltTestBase.setUp() generates/caches large video fixtures and edge-case files; StreamsPickerTest appears to only exercise pure selection logic and metadata parsing. Inheriting from MeltTestBase makes these unit tests pay an unnecessary setup cost (and can be very slow in CI without a persistent cache). Consider inheriting from TwoToneTestCase (or introducing a lightweight base) for this file, reserving MeltTestBase for integration tests that actually need sample_video_file / edge_fixtures.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-04T18:07:15Z

+
+# Ensure tests/ is on sys.path so that test modules in subdirectories
+# can use ``from common import ...`` the same way top-level tests do.
+sys.path.insert(0, os.path.dirname(__file__))


Unconditionally inserting tests/ at the front of sys.path can shadow similarly named top-level modules (e.g., common, melt, etc.) and can also lead to duplicate entries if imported multiple times. A safer pattern is to (1) only insert if not already present and/or (2) prefer package-relative imports inside tests/ (e.g., from .helpers import ...) to avoid global import side-effects.

Suggested change

sys.path.insert(0, os.path.dirname(__file__))

tests_dir = os.path.dirname(__file__)

if tests_dir not in sys.path:

sys.path.insert(0, tests_dir)

Copilot · 2026-04-04T18:07:16Z

    for media in ["subtitles", "subtitles_txt", "videos"]:
-        for root, _, files in os.walk(os.path.join(current_path, media)):
+        for root, _, files in os.walk(os.path.join(media_path, media)):


add_test_media() now searches for subtitles and subtitles_txt under media_path (i.e., tests/media/...). In this PR, .gitmodules only moves videos, audio, and images into tests/media/, so unless subtitle fixtures were also moved to tests/media/subtitles*, this change will break test fixture discovery for subtitles. If subtitles are not under tests/media/, consider keeping subtitles/subtitles_txt rooted at current_path while only using media_path for submodule-backed assets.

Copilot · 2026-04-04T18:07:16Z

+            #unify fps and add audio path
+            output_dir = os.path.join(self.wd.path, "gen_sample")
+
+            with ScopedDirectory(output_dir) as sd:


The context manager variable sd is never used. If this repository enforces unused-variable checks, this can fail linting (e.g., ruff/flake8 F841). If you don't need the bound value, use with ScopedDirectory(output_dir):.

Suggested change

with ScopedDirectory(output_dir) as sd:

with ScopedDirectory(output_dir):

Copilot · 2026-04-04T18:07:16Z

@@ -0,0 +1 @@
+


These newly added __init__.py files appear to contain whitespace-only content (a single blank line). If lint rules like W293 blank line contains whitespace are enabled, this can fail. Prefer truly empty __init__.py files (0 bytes) or at least ensure the line has no trailing spaces. This applies to the other newly added tests/**/__init__.py files showing the same pattern.

Kicer86 changed the title ~~Tests~~ Reorganize tests Mar 9, 2026

Kicer86 added 5 commits April 4, 2026 11:14

Add test infrastructure for subdirectory organization

d72f7a4

Add conftest.py that ensures tests/ is on sys.path so test modules in subdirectories can import from common. Add __init__.py files to all tool-specific test subdirectories.

Move utils tests into tests/utils/ subdirectory

dfe002b

Relocate utility test files: - test_utils.py → utils/test_video_utils.py - test_requirements_utils.py → utils/test_requirements_utils.py

Kicer86 force-pushed the tests branch from 62a1912 to 4a8a072 Compare April 4, 2026 14:01

Kicer86 added 10 commits April 4, 2026 16:02

Use test logger in melt integration tests

a25d59b

Remove unused unittest import from melt helpers

8e7836c

Remove unused PhashCache import from pair matcher unit tests

0f75a16

Use pathlib in conftest.py

105841d

Fix mutable default argument in run_twotone

356012e

Use raw string for Windows path literal in streams picker test

c7f7c92

Revert "Use pathlib in conftest.py"

2a82c89

This reverts commit 105841d.

Align .gitmodules submodule names with actual paths

e505249

Kicer86 marked this pull request as ready for review April 4, 2026 18:04

Kicer86 requested a review from Copilot April 4, 2026 18:04

Copilot AI reviewed Apr 4, 2026

View reviewed changes

Kicer86 merged commit ec15953 into master Apr 5, 2026
10 of 11 checks passed

Kicer86 deleted the tests branch April 5, 2026 06:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reorganize tests#121

Reorganize tests#121
Kicer86 merged 15 commits into
masterfrom
tests

Kicer86 commented Mar 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 4, 2026

Uh oh!

Copilot AI Apr 4, 2026

Uh oh!

Copilot AI Apr 4, 2026

Uh oh!

Copilot AI Apr 4, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-sys.path.insert(0, os.path.dirname(__file__))
+tests_dir = os.path.dirname(__file__)
+if tests_dir not in sys.path:
+    sys.path.insert(0, tests_dir)

	with ScopedDirectory(output_dir) as sd:
	with ScopedDirectory(output_dir):

Conversation

Kicer86 commented Mar 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants