[FEATURE] Add WebVTT regression test coverage #993
+309
−17
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Rahul-2k4
requested review from
canihavesomecoffee and
thealphadollar
as code owners
Author
|
Hi @canihavesomecoffee , |
- Add 'Output Formats' category for format-specific tests - Fix misleading description to 'Validates WebVTT header generation on empty-caption input' - Add explicit WEBVTT_TEST_ID constant to prevent fragile ordering bugs - Update expected output to match exact line endings (CRLF) - Add GOLDEN_FILE_PROVENANCE.md documenting reproducibility
The correct field in RegressionTestOutput should match the actual sample1.webvtt file content (WEBVTT\r\n\r\n) for consistency.
Bumps [mypy](https://github.com/python/mypy) from 1.5.1 to 1.19.1. - [Changelog](https://github.com/python/mypy/blob/master/CHANGELOG.md) - [Commits](python/mypy@v1.5.1...v1.19.1) --- updated-dependencies: - dependency-name: mypy dependency-version: 1.19.1 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]>
This PR removes the last two blocking wait_for_operation calls that were causing gunicorn workers to be blocked for extended periods: 1. delete_expired_instances() - VM deletion is now fire-and-forget 2. start_test() - VM creation is now optimistic (recorded immediately) These blocking calls were causing 504 timeouts on webhook deliveries because GitHub has a 10-second webhook timeout. When cron jobs were running wait_for_operation (which can take up to 30 minutes), all gunicorn workers could become blocked, causing webhook requests to queue and exceed the timeout. Changes: - delete_expired_instances: Remove blocking wait, log operation initiation - start_test: Check for immediate errors, then record instance optimistically - Tests: Remove unused wait_for_operation mocks from affected tests If VM creation ultimately fails, the test won't report progress and will be cleaned up by the expired instances cron job. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Implements a robust system to ensure GCP VMs are properly deleted: - Add PendingDeletion model to track deletion operations - Add delete_instance_with_tracking() to record pending deletions - Add verify_pending_deletions() to check operation status and retry failures - Add scan_for_orphaned_vms() as final safety net for missed deletions - Integrate verification into start_platforms() cron flow - Update both deletion points to use tracking - Add database migration for pending_deletion table - Add 11 tests for new functionality This prevents orphaned VMs from continuing to incur billing charges when deletion operations fail silently. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
The appleboy/ssh-action no longer supports the `script_stop` parameter. Replace with `set -e` in scripts that need to stop on first error. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
The health check was failing with 301 because nginx redirects HTTP to HTTPS. Adding -L makes curl follow the redirect and get the actual health endpoint response. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Bumps [xmltodict](https://github.com/martinblech/xmltodict) from 0.13.0 to 1.0.2. - [Release notes](https://github.com/martinblech/xmltodict/releases) - [Changelog](https://github.com/martinblech/xmltodict/blob/master/CHANGELOG.md) - [Commits](martinblech/xmltodict@v0.13.0...v1.0.2) --- updated-dependencies: - dependency-name: xmltodict dependency-version: 1.0.2 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]>
Bumps [coverage](https://github.com/coveragepy/coveragepy) from 7.13.0 to 7.13.1. - [Release notes](https://github.com/coveragepy/coveragepy/releases) - [Changelog](https://github.com/coveragepy/coveragepy/blob/main/CHANGES.rst) - [Commits](coveragepy/coveragepy@7.13.0...7.13.1) --- updated-dependencies: - dependency-name: coverage dependency-version: 7.13.1 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]>
Bumps [lxml](https://github.com/lxml/lxml) from 5.3.0 to 6.0.2. - [Release notes](https://github.com/lxml/lxml/releases) - [Changelog](https://github.com/lxml/lxml/blob/master/CHANGES.txt) - [Commits](lxml/lxml@lxml-5.3.0...lxml-6.0.2) --- updated-dependencies: - dependency-name: lxml dependency-version: 6.0.2 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]>
Bumps [google-api-python-client](https://github.com/googleapis/google-api-python-client) from 2.154.0 to 2.187.0. - [Release notes](https://github.com/googleapis/google-api-python-client/releases) - [Commits](googleapis/google-api-python-client@v2.154.0...v2.187.0) --- updated-dependencies: - dependency-name: google-api-python-client dependency-version: 2.187.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]>
Bumps [gitpython](https://github.com/gitpython-developers/GitPython) from 3.1.45 to 3.1.46. - [Release notes](https://github.com/gitpython-developers/GitPython/releases) - [Changelog](https://github.com/gitpython-developers/GitPython/blob/main/CHANGES) - [Commits](gitpython-developers/GitPython@3.1.45...3.1.46) --- updated-dependencies: - dependency-name: gitpython dependency-version: 3.1.46 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]>
This commit introduces a progress indicator that displays the number of completed samples and percentage during the 'Testing' stage of test execution, enhancing user visibility into test progress. Changes: - Modified mod_test/controllers.py to calculate and return sample progress data in the get_json_data endpoint - Updated templates/test/by_id.html to display progress information and handle real-time updates via AJAX polling Implementation details: - Calculates progress using existing test.results and test.get_customized_regressiontests() data - No database schema changes required - Updates every 20 seconds via existing AJAX mechanism - Displays format: 'X / Y samples (Z%)' - Backward compatible with existing functionality The feature provides immediate feedback to users about test execution progress without requiring any infrastructure changes.
The old message claimed tests were queued sequentially with estimated wait times based on queue position. This was inaccurate - tests actually run in parallel on separate GCP VMs. Changes: - Replace "X tests queued before this one" with "Your test VM is being provisioned" - Show average completion time for the platform instead of fake queue math - Remove unused imports (datetime, func, label, GcpInstance) - Simplify get_data_for_test() by removing complex queue calculations 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
The test was checking for g.db.query calls that no longer exist after removing the queue calculation logic. Updated to verify the new behavior that only queries GeneralData for average times. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Previously the test page hardcoded "0 / 0 samples" and only updated via AJAX after 20 seconds. Now the sample progress is calculated on initial render so users see accurate progress immediately. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
When the log file is owned by a different user (e.g., root vs www-data), the app would crash on startup with PermissionError. This was causing workers to fail immediately. Changes: - Catch PermissionError and OSError when creating the file handler - Fall back to console-only logging instead of crashing - Print helpful warning message with fix instructions to stderr - Update file_logger property to return Optional[RotatingFileHandler] - Update create_logger to skip file handler when unavailable This prevents the entire application from crashing due to a log file permission issue, allowing it to continue operating with console logging while still alerting operators to fix the underlying permission problem. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Per review feedback: ensure logs directory is owned by www-data during deployment, preventing silent fallback to console-only logging. The pre-deploy script now: - Checks if logs directory has correct ownership - Automatically fixes ownership if possible (when run as root) - Fails deployment if ownership cannot be fixed 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
This PR upgrades SQLAlchemy to version 2.0.45, addressing breaking changes: - Update DeclarativeBase: Replace deprecated declarative_base() with new class-based DeclarativeBase pattern - Fix raw SQL execution: Wrap string SQL with text() in mod_health and tests - Update DeclEnumType: Use String(50) as impl instead of Enum to avoid SQLAlchemy 2.0's stricter enum value validation in TypeDecorator - Fix engine creation: Remove deprecated convert_unicode parameter and use StaticPool for SQLite in-memory databases to ensure connection sharing - Update type hints: Use generic Dialect type instead of SQLite-specific All 402 tests pass with the upgraded SQLAlchemy version. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Keep EnumSymbol and str types instead of Any for better type safety. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
In Python 3.13+, MagicMock auto-detects async-like method names (commit, debug, warning, etc.) and returns AsyncMock objects. This causes RuntimeWarnings when these methods are called synchronously. This commit adds: - A warning filter to suppress "coroutine was never awaited" warnings - A setup_mock_g() helper for tests that need explicit MagicMock setup - Updated create_mock_db_query() to also set up g.log 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
The code in mod_ci/controllers.py:1785 expects status objects with a .context attribute, but the test was returning dictionaries. This worked in earlier Python versions but fails in Python 3.14. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
- test_add_test_empty_erc: Changed assertion to check id=4 (id=3 now used by WebVTT test) - test_update_expected_results_: Updated expected test count from 2 to 3 - sample1.webvtt: Fixed line endings to use CRLF as expected by code
Rahul-2k4
force-pushed
the
feature/webvtt-regression-test
branch
from
20bc907 to
555acfc
Compare
- mod_ci/models.py: Add ClassVar type annotation to MAX_RETRIES (RUF012) - tests/test_log_configuration.py: Add missing file_logger assertion - templates/test/by_id.html: Fix AJAX percentage display and use jQuery - tests/base.py: Add 'Output Formats' category and update WebVTT test
The test_add_test was checking for regression test ID=3, which now belongs to the pre-existing WebVTT test. The assertion was passing for the wrong reason (WebVTT exists) rather than verifying the newly created test. Updated to check ID=4 which is the correct ID for the newly created test.
|
9 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In raising this pull request, I confirm the following (please check boxes):
My familiarity with the project is as follows (check one):
Summary
-out=webvtt)sample1.webvttfor test validationTest plan