Skip to content

Conversation

@google-labs-jules
Copy link
Contributor

This change adds a comprehensive suite of unit tests for the ydotool_injector.rs module. It introduces a TestHarness to create an isolated test environment, mocking the filesystem, environment variables, and external binaries. The ydotool_injector.rs logic was made testable by introducing a UINPUT_PATH_OVERRIDE environment variable to avoid permission issues with /dev/uinput during tests. The tests cover socket path discovery logic, binary permission and availability checks, and inject_text functionality, including the fallback from 'paste' to 'type' mode.

Fixes #265


PR created automatically by Jules for task 18412184917474101525 started by @Coldaine

This commit introduces a full suite of unit tests for the `ydotool_injector.rs` module.

- A `TestHarness` is implemented to create an isolated test environment, mocking the filesystem, environment variables, and external binaries (`ydotool`, `which`).
- The core `ydotool_injector.rs` logic was made testable by introducing a `UINPUT_PATH_OVERRIDE` environment variable under a `cfg(test)` flag to avoid permission issues with `/dev/uinput` during tests.
- Tests cover:
  - Socket path discovery logic.
  - Binary permission and availability checks.
  - `inject_text` functionality, including the fallback from 'paste' to 'type' mode.

These tests improve the reliability and maintainability of the ydotool injection backend.
@google-labs-jules
Copy link
Contributor Author

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!


For security, I will only act on instructions from the user who triggered this task.

New to Jules? Learn more at jules.google/docs.

@qodo-code-review
Copy link

qodo-code-review bot commented Dec 3, 2025

CI Feedback 🧐

(Feedback updated until commit 1efe36c)

A test triggered by this PR failed. Here is an AI-generated analysis of the failure:

Action: Lint & Format

Failed stage: Check formatting [❌]

Failure summary:

The action failed because the code formatting check (cargo fmt --all -- --check) detected formatting
inconsistencies in the codebase. The formatter found multiple files with formatting differences:

- crates/coldvox-stt/src/plugins/parakeet.rs (lines 287, 362)
- crates/coldvox-stt/src/processor.rs
(lines 174, 220)
- crates/coldvox-telemetry/src/stt_metrics.rs (lines 99, 227, 250, 309, 347)
-
crates/coldvox-text-injection/src/tests/test_ydotool_injector.rs (line 157)

The differences include:
- Incorrect indentation and line breaks
- Inconsistent spacing in struct
field definitions
- Improper formatting of function calls and closures
- Missing or extra blank
lines

The code needs to be reformatted using cargo fmt to match the project's formatting standards.

Relevant error logs:
1:  Runner name: 'laptop-extra'
2:  Runner group name: 'Default'
...

137:  CARGO_TERM_COLOR: always
138:  targets: 
139:  components: rustfmt, clippy
140:  ##[endgroup]
141:  ##[group]Run : set $CARGO_HOME
142:  �[36;1m: set $CARGO_HOME�[0m
143:  �[36;1mecho CARGO_HOME=${CARGO_HOME:-"$HOME/.cargo"} >> $GITHUB_ENV�[0m
144:  shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
145:  env:
146:  RUSTFLAGS: -D warnings
147:  CARGO_TERM_COLOR: always
148:  ##[endgroup]
149:  ##[group]Run : install rustup if needed
150:  �[36;1m: install rustup if needed�[0m
151:  �[36;1mif ! command -v rustup &>/dev/null; then�[0m
152:  �[36;1m  curl --proto '=https' --tlsv1.2 --retry 10 --retry-connrefused --location --silent --show-error --fail https://sh.rustup.rs | sh -s -- --default-toolchain none -y�[0m
153:  �[36;1m  echo "$CARGO_HOME/bin" >> $GITHUB_PATH�[0m
...

224:  �[36;1m  if rustc +stable --version --verbose | grep -q '^release: 1\.6[89]\.'; then�[0m
225:  �[36;1m    touch "/home/coldaine/actions-runner/_work/_temp"/.implicit_cargo_registries_crates_io_protocol || true�[0m
226:  �[36;1m    echo CARGO_REGISTRIES_CRATES_IO_PROTOCOL=sparse >> $GITHUB_ENV�[0m
227:  �[36;1m  elif rustc +stable --version --verbose | grep -q '^release: 1\.6[67]\.'; then�[0m
228:  �[36;1m    touch "/home/coldaine/actions-runner/_work/_temp"/.implicit_cargo_registries_crates_io_protocol || true�[0m
229:  �[36;1m    echo CARGO_REGISTRIES_CRATES_IO_PROTOCOL=git >> $GITHUB_ENV�[0m
230:  �[36;1m  fi�[0m
231:  �[36;1mfi�[0m
232:  shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
233:  env:
234:  RUSTFLAGS: -D warnings
235:  CARGO_TERM_COLOR: always
236:  CARGO_HOME: /home/coldaine/.cargo
237:  CARGO_INCREMENTAL: 0
238:  ##[endgroup]
239:  ##[group]Run : work around spurious network errors in curl 8.0
240:  �[36;1m: work around spurious network errors in curl 8.0�[0m
241:  �[36;1m# https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/timeout.20investigation�[0m
...

328:  - /home/coldaine/actions-runner/_work/ColdVox/ColdVox/crates/coldvox-telemetry/Cargo.toml
329:  - /home/coldaine/actions-runner/_work/ColdVox/ColdVox/crates/coldvox-text-injection/Cargo.toml
330:  - /home/coldaine/actions-runner/_work/ColdVox/ColdVox/crates/coldvox-vad-silero/Cargo.toml
331:  - /home/coldaine/actions-runner/_work/ColdVox/ColdVox/crates/coldvox-vad/Cargo.toml
332:  ##[endgroup]
333:  ... Restoring cache ...
334:  No cache found.
335:  ##[group]Run cargo fmt --all -- --check
336:  �[36;1mcargo fmt --all -- --check�[0m
337:  shell: /usr/bin/bash -e {0}
338:  env:
339:  RUSTFLAGS: -D warnings
340:  CARGO_TERM_COLOR: always
341:  CARGO_HOME: /home/coldaine/.cargo
342:  CARGO_INCREMENTAL: 0
343:  CACHE_ON_FAILURE: false
344:  ##[endgroup]
...

359:  confidence_scores: true,
360:  speaker_diarization: false, // Can be added later via pyannote
361:  -            auto_punctuation: true, // Both variants support punctuation
362:  +            auto_punctuation: true,     // Both variants support punctuation
363:  custom_vocabulary: false,
364:  }
365:  }
366:  Diff in /home/coldaine/actions-runner/_work/ColdVox/ColdVox/crates/coldvox-stt/src/plugins/parakeet.rs:287:
367:  };
368:  // Initialize the model
369:  -            let model = Parakeet::from_pretrained(self.variant.model_identifier(), Some(parakeet_config))
370:  -                .map_err(|err| {
371:  +            let model =
372:  +                Parakeet::from_pretrained(self.variant.model_identifier(), Some(parakeet_config))
373:  +                    .map_err(|err| {
374:  error!(
375:  target: "coldvox::stt::parakeet",
376:  error = %err,
377:  Diff in /home/coldaine/actions-runner/_work/ColdVox/ColdVox/crates/coldvox-stt/src/processor.rs:174:
378:  /// Handle speech end event
379:  async fn handle_speech_end(&mut self, _timestamp_ms: u64, _duration_ms: Option<u64>) {
380:  debug!(target: "stt", "Starting handle_speech_end()");
381:  -        let _guard = coldvox_telemetry::TimingGuard::new(
382:  -            &self.metrics,
383:  -            |m, d| m.record_end_to_end_latency(d)
384:  -        );
385:  +        let _guard = coldvox_telemetry::TimingGuard::new(&self.metrics, |m, d| {
386:  +            m.record_end_to_end_latency(d)
387:  +        });
388:  if let UtteranceState::SpeechActive { audio_buffer, .. } = &self.state {
389:  if !audio_buffer.is_empty() {
390:  Diff in /home/coldaine/actions-runner/_work/ColdVox/ColdVox/crates/coldvox-stt/src/processor.rs:220:
391:  TranscriptionEvent::Error { .. } => self.metrics.record_error(),
392:  }
393:  -        if tokio::time::timeout(
394:  -            std::time::Duration::from_secs(5),
395:  -            self.event_tx.send(event),
396:  -        )
397:  -        .await
398:  -        .is_err()
399:  +        if tokio::time::timeout(std::time::Duration::from_secs(5), self.event_tx.send(event))
400:  +            .await
401:  +            .is_err()
402:  {
403:  self.metrics.record_error();
404:  debug!(target: "stt", "Event channel closed or send timed out");
...

433:  +        "  Preprocessing:   {:.1}ms",
434:  +        latency.preprocessing_us as f64 / 1000.0
435:  +    );
436:  +    println!(
437:  +        "  Result Delivery: {:.1}ms",
438:  +        latency.result_delivery_us as f64 / 1000.0
439:  +    );
440:  // Accuracy metrics
441:  let avg_confidence = metrics.get_average_confidence();
442:  Diff in /home/coldaine/actions-runner/_work/ColdVox/ColdVox/crates/coldvox-telemetry/src/stt_metrics.rs:99:
443:  /// Performance alert types
444:  #[derive(Debug, Clone, PartialEq)]
445:  pub enum PerformanceAlert {
446:  -    HighLatency { measured_us: u64, threshold_us: u64 },
447:  -    LowConfidence { measured: f64, threshold: f64 },
448:  -    HighErrorRate { measured_per_1k: u64, threshold_per_1k: u64 },
449:  -    HighMemoryUsage { measured_bytes: u64, threshold_bytes: u64 },
450:  -    ProcessingStalled { last_activity: Duration },
451:  +    HighLatency {
452:  +        measured_us: u64,
453:  +        threshold_us: u64,
454:  +    },
455:  +    LowConfidence {
456:  +        measured: f64,
457:  +        threshold: f64,
458:  +    },
459:  +    HighErrorRate {
460:  +        measured_per_1k: u64,
461:  +        threshold_per_1k: u64,
462:  +    },
463:  +    HighMemoryUsage {
464:  +        measured_bytes: u64,
465:  +        threshold_bytes: u64,
466:  +    },
467:  +    ProcessingStalled {
468:  +        last_activity: Duration,
469:  +    },
470:  }
471:  impl SttPerformanceMetrics {
472:  Diff in /home/coldaine/actions-runner/_work/ColdVox/ColdVox/crates/coldvox-telemetry/src/stt_metrics.rs:227:
473:  }
474:  let total_ops = inner.operational.request_count;
475:  -        if total_ops > 1000 { // Only check if significant number of operations
476:  -             let error_rate_per_1k = (inner.operational.error_count * 1000) / total_ops;
477:  -             if error_rate_per_1k > thresholds.max_error_rate_per_1k {
478:  +        if total_ops > 1000 {
479:  +            // Only check if significant number of operations
480:  +            let error_rate_per_1k = (inner.operational.error_count * 1000) / total_ops;
481:  +            if error_rate_per_1k > thresholds.max_error_rate_per_1k {
482:  alerts.push(PerformanceAlert::HighErrorRate {
483:  measured_per_1k: error_rate_per_1k,
484:  threshold_per_1k: thresholds.max_error_rate_per_1k,
485:  Diff in /home/coldaine/actions-runner/_work/ColdVox/ColdVox/crates/coldvox-telemetry/src/stt_metrics.rs:250:
...

523:  inner.resources.clone(),
524:  -            inner.operational.clone()
525:  +            inner.operational.clone(),
526:  )
527:  }
528:  }
529:  Diff in /home/coldaine/actions-runner/_work/ColdVox/ColdVox/crates/coldvox-telemetry/src/stt_metrics.rs:309:
530:  }
531:  }
532:  -
533:  #[cfg(test)]
534:  mod tests {
535:  use super::*;
536:  Diff in /home/coldaine/actions-runner/_work/ColdVox/ColdVox/crates/coldvox-telemetry/src/stt_metrics.rs:347:
537:  metrics.record_transcription_success();
538:  metrics.record_transcription_failure();
539:  let rate = metrics.get_success_rate();
...

591:  harness.create_which_mock(&ydotool_path).unwrap();
592:  let socket_path = harness.home_dir.join(".ydotool/socket");
593:  harness.create_mock_socket(&socket_path).unwrap();
594:  Diff in /home/coldaine/actions-runner/_work/ColdVox/ColdVox/crates/coldvox-text-injection/src/tests/test_ydotool_injector.rs:157:
595:  #[serial]
596:  async fn test_inject_text_uses_paste_by_default() {
597:  let harness = TestHarness::new().unwrap();
598:  -    let ydotool_script = format!(
599:  -        "#!/bin/sh\necho \"$@\" > {}",
600:  -        harness.output_file.display()
601:  -    );
602:  +    let ydotool_script = format!("#!/bin/sh\necho \"$@\" > {}", harness.output_file.display());
603:  let ydotool_path = harness
604:  .create_mock_binary("ydotool", &ydotool_script, true)
605:  .unwrap();
606:  ##[error]Process completed with exit code 1.
607:  Post job cleanup.

Coldaine pushed a commit that referenced this pull request Dec 23, 2025
Rebased from Jules draft #273.
- Add TestHarness for controlled test environment
- Add tests for socket discovery, binary permissions, text injection
- Gate tests behind ydotool feature flag
@Coldaine
Copy link
Owner

Superseded by #311 (rebased and integrated)

@Coldaine Coldaine closed this Dec 24, 2025
@google-labs-jules
Copy link
Contributor Author

Superseded by #311 (rebased and integrated)

Understood. Acknowledging that this work is now obsolete and stopping work on this task.

Coldaine pushed a commit that referenced this pull request Dec 25, 2025
Rebased from Jules draft #273.
- Add TestHarness for controlled test environment
- Add tests for socket discovery, binary permissions, text injection
- Gate tests behind ydotool feature flag
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.

Add unit tests for ydotool injector

2 participants