Skip to content

[IMPROVEMENT]: Add ATSC XMLTV regression test coverage via Alembic migration - #1003

Open
x15sr71 wants to merge 4 commits into
CCExtractor:masterfrom
x15sr71:sp/add-xmltv-regression-test
Open

[IMPROVEMENT]: Add ATSC XMLTV regression test coverage via Alembic migration#1003
x15sr71 wants to merge 4 commits into
CCExtractor:masterfrom
x15sr71:sp/add-xmltv-regression-test

Conversation

@x15sr71

@x15sr71 x15sr71 commented Jan 9, 2026

Copy link
Copy Markdown
Contributor

In raising this pull request, I confirm the following (please check boxes):

  • I have read and understood the contributors guide.
  • I have checked that another pull request for this purpose does not exist.
  • I have considered, and confirmed that this submission will be valuable to others.
  • I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
  • I give this submission freely, and claim no ownership to its content.

My familiarity with the project is as follows (check one):

  • I have never used the project.
  • I have used the project briefly.
  • I have used the project extensively, but have not contributed previously.
  • I am an active contributor to the project.

Summary

Add regression test coverage for ATSC XMLTV output generation (--xmltv=1) using the ATSC broadcast sample I uploaded to the platform.

This test exercises:

  • ATSC PSI/SI table parsing (VCT, ETT, EIT)
  • XMLTV format generation from broadcast EPG data
  • Exit code validation (expected_rc=10) with OutputType.null

Technical Details

Sample Selection

Sample 187 (ch29FullTS.ts) – ATSC broadcast sample for XMLTV testing with --xmltv=1 --out=null:

  • SHA256: 6b10f8aef895469ec9cb89ecd0c2463eb2442052c32da9d81c6a6f5cec9137b8
  • Size: 300.52 MB | Duration: 2m 10s
  • ATSC PSI/SI Tables:
    • Virtual Channel Table (VCT) - channels 5.1-5.5 (KPIX-TV, StartTV, etc.)
    • Extended Text Table (ETT) - program titles/descriptions
    • Event Information Table (EIT) - multi-day EPG scheduling
  • Captions: EIA-608/708 across 5 video PIDs

I reviewed the TS samples currently available on the platform and did not find any that contain the ATSC PSI/SI tables (VCT, EIT, ETT) needed to exercise the ATSC XMLTV generation paths. For this reason, I recently added an ATSC broadcast sample to provide regression coverage.

Database Deployment Strategy

Following the pattern from the WebVTT PR (#993):

Environment Sample Used Mechanism Purpose
Production Sample 187 Alembic migration Real ATSC XMLTV validation
Fresh install Samples 1-2 install/sample_db.py Bootstrap without FK errors
CI Tests Samples 1-2 tests/base.py fixtures Fast smoke tests

Why no test fixture changes:

  • Sample 187 is a 300MB production upload (not in test fixtures)
  • Adding it would cause foreign key violations in test DB
  • Migration-only registration follows the existing pattern for uploaded samples

Test Characteristics

  • OutputType.null: Exit-code-only validation (no golden file comparison)
  • expected_rc = 10: Correct ATSC XMLTV processing return code
  • ignore = false: Standard validation (no special error handling)
  • Category: "General" - Uses existing category for organization
  • Idempotent: Migration checks for existing test before inserting (safe to re-run)

Migration Details

  • Revision: eb7303e132c0
  • Revises: c8f3a2b1d4e5 (latest migration on master)
  • Creates: Regression test + category linkage + output metadata
  • Downgrade: Clean removal in correct FK order

Related Work

@sonarqubecloud

Copy link
Copy Markdown

@cfsmp3
cfsmp3 self-requested a review June 24, 2026 17:11

@cfsmp3 cfsmp3 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

  • Migration-head collision: revises head c8f3a2b1d4e5 — but so does #1071. Second to merge → two alembic heads → flask db upgrade breaks.
  • Contradictory output row: correct='', ignore=0 will perpetually diff, yet the PR claims "exit-code-only" validation. Pick one (set ignore=1, or store a real hash).

x15sr71 added a commit to x15sr71/sample-platform that referenced this pull request Jul 29, 2026
The migration listed "from alembic import op" before "import sqlalchemy as sa"; isort (repo default) wants the straight import first, matching every other migration. Fixes the failing "Apply isort" build step on PR CCExtractor#1003. Pre-existing since the migration was authored (d03b808), not introduced by the ignore/backtick fix.
x15sr71 added a commit to x15sr71/sample-platform that referenced this pull request Jul 29, 2026
The migration listed "from alembic import op" before "import sqlalchemy as sa"; isort (repo default) wants the straight import first, matching every other migration. Fixes the failing "Apply isort" build step on PR CCExtractor#1003. Pre-existing since the migration was authored (d03b808), not introduced by the ignore/backtick fix.
@x15sr71
x15sr71 force-pushed the sp/add-xmltv-regression-test branch from 9e83a6d to 24df5fc Compare July 29, 2026 19:44
x15sr71 added a commit to x15sr71/sample-platform that referenced this pull request Jul 29, 2026
The migration listed "from alembic import op" before "import sqlalchemy as sa"; isort (repo default) wants the straight import first, matching every other migration. Fixes the failing "Apply isort" build step on PR CCExtractor#1003. Pre-existing since the migration was authored (d03b808), not introduced by the ignore/backtick fix.
@x15sr71
x15sr71 force-pushed the sp/add-xmltv-regression-test branch from 24df5fc to fcb85c9 Compare July 29, 2026 19:47
x15sr71 added a commit to x15sr71/sample-platform that referenced this pull request Jul 29, 2026
The migration listed "from alembic import op" before "import sqlalchemy as sa"; isort (repo default) wants the straight import first, matching every other migration. Fixes the failing "Apply isort" build step on PR CCExtractor#1003. Pre-existing since the migration was authored (d03b808), not introduced by the ignore/backtick fix.
@x15sr71
x15sr71 force-pushed the sp/add-xmltv-regression-test branch from fcb85c9 to a30567f Compare July 29, 2026 20:11
x15sr71 added 4 commits July 30, 2026 01:47
The migration listed "from alembic import op" before "import sqlalchemy as sa"; isort (repo default) wants the straight import first, matching every other migration. Fixes the failing "Apply isort" build step on PR CCExtractor#1003. Pre-existing since the migration was authored (d03b808), not introduced by the ignore/backtick fix.
@x15sr71
x15sr71 force-pushed the sp/add-xmltv-regression-test branch from a30567f to db74635 Compare July 29, 2026 20:18
@sonarqubecloud

Copy link
Copy Markdown

@x15sr71

x15sr71 commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review and suggesting the fix — fixed the output-row contradiction and found a MySQL-only blocker, fixed it too:

  • Output row (fixed): set ignore=1 → exit-code-only, matching the PR's intent. Checked against CCExtractorTester: with ignore=1 it reports an exit-code-only result and never compares output, so there's no perpetual diff.
  • ignore reserved word (new — blocker): the raw INSERT listed ignore unquoted, which passes on SQLite (CI) but fails with ERROR 1064 on MySQL/prod. Backticked it.

Head collision: If #1071 merges first, I'm happy to rebase this PR onto it. Just let me know your preferred order.

@x15sr71
x15sr71 requested a review from cfsmp3 July 29, 2026 20:33
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.

2 participants