-
Notifications
You must be signed in to change notification settings - Fork 0
fix: restore STT CI coverage and fix GTK test race #309
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
|||||||||||||||||||||||||||||||||||||||||||||
ⓘ Your approaching your monthly quota for Qodo. Upgrade your plan PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
|||||||||||||||||||||||||||||||||||||
498ffd0 to
21e30e9
Compare
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||
ⓘ Your approaching your monthly quota for Qodo. Upgrade your plan PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||||||||
qodo-code-review
bot
commented
Dec 23, 2025
•
edited by qodo-free-for-open-source-projects
bot
Loading
edited by qodo-free-for-open-source-projects
bot
CI Feedback 🧐(Feedback updated until commit 21e30e9)A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
ⓘ Your approaching your monthly quota for Qodo. Upgrade your plan CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR addresses two critical issues in the CI/CD pipeline and test infrastructure: restoring STT test coverage for the Moonshine model and fixing a race condition in the GTK test harness.
Key Changes:
- Added dedicated CI step to run Moonshine E2E tests with required Python dependencies
- Fixed GTK test app race condition by deferring ready file creation to the main loop
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
.github/workflows/ci.yml |
Added new CI step to run Moonshine E2E tests with Python dependencies (transformers, torch, librosa, accelerate) and moonshine feature flag enabled |
crates/coldvox-text-injection/test-apps/gtk_test_app.c |
Fixed race condition by converting create_ready_file to an idle callback and scheduling it with g_idle_add to ensure the main loop is running before signaling readiness |
| run: | | ||
| echo "=== Running Moonshine E2E Tests ===" | ||
| # Install Python dependencies for Moonshine | ||
| pip install transformers torch librosa accelerate |
Copilot
AI
Dec 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Python dependencies are installed without version pinning, which can lead to non-reproducible builds and potential security issues if a dependency is compromised. Consider pinning versions (e.g., pip install transformers==4.36.0 torch==2.1.2 librosa==0.10.1 accelerate==0.25.0) or using a requirements file to ensure consistent test environments across runs.
|
The |
|
| Severity | Issue | Location |
|---|---|---|
| WARNING | Missing error handling in create_ready_file function | crates/coldvox-text-injection/test-apps/gtk_test_app.c:24-30 |
Recommendation: Address the warning issue before merge
Review Details (2 files)
Files: .github/workflows/ci.yml, crates/coldvox-text-injection/test-apps/gtk_test_app.c
…eady file creation (secure, logged, PID content)
|
Thanks for the thorough review — I’ve addressed the key points:
Synopsis xdg-open { file | URL } xdg-open { --help | --manual | --version } Use 'man xdg-open' or 'xdg-open --manual' for additional info. with , mode ), added error logging via perror Ver 2.11, for Linux (x86_64) Print a description for a system error code or a MariaDB error code. Usage: perror [OPTIONS] [ERRORCODE [ERRORCODE...]] Variables (--variable-name=value) verbose TRUE, and wrote the current PID into the file for harness verification. See gtk_test_app.c.
If there are any remaining blockers, let me know and I’ll address promptly. |
|
Thanks for the thorough review — I have addressed the key points:
If there are any remaining blockers, let me know and I will address promptly. |
User description
Summary
This PR addresses critical gaps identified during the post-merge review of the CI/CD pipeline and test harness.
Changes
CI / STT Coverage:
Run Moonshine E2E Testsstep to the hosted CI runner.transformers,torch,librosa,accelerate) which were previously missing, causing tests to be skipped or fail silently.moonshinefeature flag for the test run.Tests / GTK Harness:
gtk_test_app.cwhere the readiness signal file was created before the main loop started.g_idle_addcallback, ensuring the app is truly responsive before tests attempt injection.Verification
moonshine_e2etests (verifying model download and inference).PR Type
Bug fix, Tests
Description
Fix GTK test race condition by deferring readiness signal
Add Moonshine E2E tests to CI with required Python dependencies
Ensure GTK app is responsive before test injection begins
Restore STT coverage in hosted CI runner
Diagram Walkthrough
File Walkthrough
gtk_test_app.c
Defer GTK readiness signal to main loop startcrates/coldvox-text-injection/test-apps/gtk_test_app.c
create_ready_file()from void function to gboolean callbackg_idle_add()scheduler
ci.yml
Add Moonshine E2E tests to CI pipeline.github/workflows/ci.yml
accelerate)