Skip to content

ci: add cargo tests#89

Merged
mayocream merged 16 commits into
mainfrom
ci
Dec 13, 2025
Merged

ci: add cargo tests#89
mayocream merged 16 commits into
mainfrom
ci

Conversation

@fffonion

Copy link
Copy Markdown
Collaborator

No description provided.

Copilot AI review requested due to automatic review settings December 13, 2025 17:36
@gemini-code-assist

Copy link
Copy Markdown

Summary of Changes

Hello @fffonion, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request focuses on adjusting the existing test suite, likely in preparation for or as part of integrating cargo test into the continuous integration pipeline. It includes refactoring a comic text detector test for better organization and debugging, updating its model loading mechanism, and temporarily disabling an FFT-related test.

Highlights

  • Test Refactoring: The comic_text_detector test has been refactored, moving it into a dedicated test module and adding tracing::instrument for improved debugging and observability.
  • Test Parameter Update: The ComicTextDetector::load call within the test was updated to simplify device selection by directly passing a boolean value.
  • Test Disablement: An existing FFT roundtrip test in lama/fft/mod.rs was temporarily commented out.
Ignored Files
  • Ignored by pattern: .github/workflows/** (5)
    • .github/workflows/build.yml
    • .github/workflows/lint.yml
    • .github/workflows/パブリッシュ.yml
    • .github/workflows/リリース.yaml
    • .github/workflows/継続的インテグレーション.yml
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request aims to add cargo tests for CI. It modifies two files: one by commenting out an existing test module, and another by refactoring an integration test. My review focuses on these changes. I've pointed out that commenting out tests is an anti-pattern and suggest removing the code or using #[ignore]. I've also suggested simplifying the integration test structure by removing a redundant module wrapper, which is not idiomatic for integration tests in Rust.

Comment thread koharu-ml/src/lama/fft/mod.rs Outdated
Comment thread koharu-ml/tests/comic_text_detector.rs Outdated

Copilot AI 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.

Pull request overview

This PR integrates cargo tests into the CI pipeline by consolidating existing workflow files and modifying the comic text detector test structure. The PR removes the separate lint.yml and build.yml workflow files in favor of a unified approach that includes running cargo test in the koharu-ml workspace.

Key changes:

  • Restructured the comic text detector test to use a test module with tracing instrumentation
  • Simplified the test by passing false directly to ComicTextDetector::load() instead of managing device selection
  • Commented out the FFT roundtrip test in the lama module

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.

File Description
koharu-ml/tests/comic_text_detector.rs Refactored test into a proper test module structure with tracing instrumentation and simplified device handling
koharu-ml/src/lama/fft/mod.rs Commented out the FFT roundtrip unit test
.github/workflows/lint.yml Deleted workflow file (consolidated into main CI workflow)
.github/workflows/build.yml Deleted workflow file (consolidated into main CI workflow)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread koharu-ml/tests/comic_text_detector.rs Outdated
Comment thread koharu-ml/src/lama/fft/mod.rs Outdated
Comment thread koharu-ml/tests/comic_text_detector.rs Outdated

Copilot AI commented Dec 13, 2025

Copy link
Copy Markdown
Contributor

@mayocream I've opened a new pull request, #90, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot AI 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.

Pull request overview

Copilot reviewed 9 out of 13 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

.github/workflows/release-packages.yaml:18

  • The job name has been changed to Japanese characters ("マイクロソフト ウィンドウズ"). Using non-English characters in CI job names can cause issues with some CI/CD tools, reduces accessibility for international teams, and makes logs harder to search. Consider using English names like "Build - Windows" for better compatibility and maintainability.
    .github/workflows/release-packages.yaml:74
  • The job name has been changed to Japanese characters ("マックオーエス"). Using non-English characters in CI job names can cause issues with some CI/CD tools, reduces accessibility for international teams, and makes logs harder to search. Consider using English names like "Build - macOS" for better compatibility and maintainability.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/workflows/lint.yml
Comment thread .github/workflows/release-packages.yaml Outdated
Comment thread .github/workflows/release-packages.yaml Outdated
Comment thread .github/workflows/build.yml Outdated
Copilot AI review requested due to automatic review settings December 13, 2025 18:21
Comment thread .github/workflows/build.yml Outdated

Copilot AI 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.

Pull request overview

Copilot reviewed 7 out of 12 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread koharu-ml/tests/ocr.rs Outdated
Comment thread koharu-ml/tests/comic_text_detector.rs Outdated
Removed test job and concurrency settings from build workflow.
Copilot AI review requested due to automatic review settings December 13, 2025 18:26
This workflow runs integration tests on pushes to the main branch and pull requests.

Copilot AI 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.

Pull request overview

Copilot reviewed 8 out of 13 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/workflows/lint.yml Outdated
Comment thread .github/workflows/lint.yml Outdated
Copilot AI review requested due to automatic review settings December 13, 2025 18:30

Copilot AI 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.

Pull request overview

Copilot reviewed 8 out of 13 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread koharu-ml/tests/comic_text_detector.rs Outdated
Copilot AI review requested due to automatic review settings December 13, 2025 18:35
mayocream and others added 2 commits December 14, 2025 03:36
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI 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.

Pull request overview

Copilot reviewed 8 out of 13 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread koharu-ml/src/lama/fft/cpu.rs
@mayocream mayocream merged commit 693bf07 into main Dec 13, 2025
3 of 4 checks passed
@mayocream mayocream deleted the ci branch December 13, 2025 18:48
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