Skip to content

[IMPROVEMENT] Test command line options through ccxr_parse_parameters into C - #2409

Open
Deepak-negi11 wants to merge 1 commit into
CCExtractor:masterfrom
Deepak-negi11:test/cli-through-c
Open

Deepak-negi11 wants to merge 1 commit into
CCExtractor:masterfrom
Deepak-negi11:test/cli-through-c

Conversation

@Deepak-negi11

@Deepak-negi11 Deepak-negi11 commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Repro instructions:

This PR only adds tests, so there's no bug to reproduce. Here's how to check that they catch something the old tests miss.

Run the tests. All 439 pass:

cd src/rust && cargo test --lib

Now break something on purpose. In src/rust/src/common.rs, delete this line, so --ucla never reaches C:

(*ccx_s_options).ucla = options.ucla as _;

Run the tests again. The other 438 still pass, and test_cases_reach_c fails with ["input.ts", "--ucla"] did not reach C as expected.


Part of #2408 (step 1 of 4).

What's wrong

The parser tests only check the Rust options right after parsing. They never run the real entry point, which also converts old spellings like -UCLA, and they never check what C gets. So bugs in those two places pass every test. #2367, #2370 and #2397 were bugs like that.

What this adds

src/rust/src/cli_tests.rs:

  • parse_to_c runs the arguments through ccxr_parse_parameters, the same function main() calls, and returns copies of ccx_options and tlt_config as C sees them. No C code runs in cargo test, so this checks what copy_from_rust writes for C. It holds a lock because those are globals, and it zeroes them before each parse so one test can't see what another left behind.
  • cases(): one table. Each entry has one or more ways to write some options (new and old spellings) and a check on the C options they give. test_cases_reach_c runs every spelling of every entry and fails if one is rejected or its check fails.
  • 10 entries: --delay, --out/--webvtt, --ucla, --608, --no-fontcolor, --gui-mode-reports, --startat, --sentencecap, --mkvlang and --tpage, most with their old spelling too.
  • Two more tests: an unknown option is rejected, and each parse starts fresh.

The tests are a table so that #2410 can count which options really have a check on the C side.

Testing

  • cargo test (439 passed), cargo fmt --check, cargo clippy -- -D warnings.
  • The break-it check above.

Copilot AI balanced review requested due to automatic review settings October 20, 2026 19:11

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@Deepak-negi11 Deepak-negi11 changed the title [IMPROVEMENT] Test command line options through ccxr_parse_parameters… [IMPROVEMENT] Test command line options through ccxr_parse_parameters into C Oct 24, 2026
@cfsmp3 cfsmp3 closed this Nov 10, 2026
@cfsmp3 cfsmp3 reopened this Nov 10, 2026
@ccextractor-bot

Copy link
Copy Markdown
Collaborator
CCExtractor CI platform finished running the test files on linux. 172/238 tests matched the approved output:
Report Name Tests Passed
Broken 9/13
CEA-708 2/14
DVB 0/7
DVD 3/3
DVR-MS 2/2
General 26/28
Hardsubx 1/1
Hauppage 3/3
MP4 3/3
NoCC 10/10
Options 69/86
Teletext 0/21
WTV 12/13
XDS 32/34

66 tests do not match the approved output. That is the pass/fail verdict. Whether this branch caused it is a separate question, answered below.


Compared with the tip of master — test 9984, commit 24d1e0a:

  • 0 pass there and fail here
  • 0 fail there and pass here
  • 0 fail on both, with different output
  • 66 fail on both, byte for byte the same

Compared with the commit this branch was cut from: the same run as the tip of master (test 9984), so the comparison above already covers it.


No test changes behaviour relative to the tip of master: every failure above fails there too, byte for byte. The approved output for those tests is out of date, which is a baseline to review rather than a regression in this branch.

@ccextractor-bot

Copy link
Copy Markdown
Collaborator
CCExtractor CI platform finished running the test files on windows. 172/238 tests matched the approved output:
Report Name Tests Passed
Broken 9/13
CEA-708 2/14
DVB 0/7
DVD 3/3
DVR-MS 2/2
General 26/28
Hardsubx 1/1
Hauppage 3/3
MP4 3/3
NoCC 10/10
Options 69/86
Teletext 0/21
WTV 12/13
XDS 32/34

66 tests do not match the approved output. That is the pass/fail verdict. Whether this branch caused it is a separate question, answered below.


Compared with the tip of master — test 9985, commit 24d1e0a:

  • 0 pass there and fail here
  • 0 fail there and pass here
  • 0 fail on both, with different output
  • 66 fail on both, byte for byte the same

Compared with the commit this branch was cut from: the same run as the tip of master (test 9985), so the comparison above already covers it.


No test changes behaviour relative to the tip of master: every failure above fails there too, byte for byte. The approved output for those tests is out of date, which is a baseline to review rather than a regression in this branch.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants