Skip to content

add ut; support BGI config - #5

Closed
LevelDownRefine wants to merge 1 commit into
mainfrom
lvdown/ut
Closed

LevelDownRefine wants to merge 1 commit into
mainfrom
lvdown/ut

Conversation

@LevelDownRefine

@LevelDownRefine LevelDownRefine commented Jun 25, 2026 •

Copy link
Copy Markdown
Owner

Summary by CodeRabbit

  • New Features

    • Added a new script to copy BetterGI configuration files into the appropriate app folder.
    • Added a dedicated config directory path used by the app and related tools.
    • Added a GitHub Actions CI workflow that runs the test suite on push and pull requests.
  • Tests

    • Added unit coverage for config path detection, file copying, and path utility behavior.

@coderabbitai

coderabbitai Bot commented Jun 25, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds BGIConfigDIR in utils.py, a script that locates BetterGI.exe from 01.yml and copies config files into its parent directory, plus unit tests and a GitHub Actions workflow that runs the test suite.

Changes

BetterGI configuration support

Layer / File(s) Summary
Path constant and utility tests
utils.py, tests/test_utils.py
BGIConfigDIR is added under BaseDIR, and the utility tests check path joining, cwd resolution, and the new constant.
BetterGI lookup, copy script, and validation
copy_bettergi_config.py, tests/test_copy_bettergi_config.py, .github/workflows/ci.yml
find_bettergi_path reads 01.yml to locate BetterGI.exe, copy_bettergi_config copies BGIConfigDIR into the install directory, the new tests cover lookup and copy behavior, and CI runs unittest discovery on Windows with uv.

Sequence Diagram(s)

sequenceDiagram
  participant Main as "__main__"
  participant Copy as "copy_bettergi_config()"
  participant Find as "find_bettergi_path()"
  participant Yaml as "yaml.safe_load()"
  participant Tree as "shutil.copytree()"
  participant FileCopy as "shutil.copy2()"

  Main->>Copy: invoke
  Copy->>Find: locate BetterGI.exe from 01.yml
  Find->>Yaml: load config_file
  Yaml-->>Find: script_list
  Find-->>Copy: BetterGI.exe path
  Copy->>Tree: copy directories from BGIConfigDIR
  Copy->>FileCopy: copy files from BGIConfigDIR
  Copy-->>Main: target directory
  Main->>Main: print os.fspath(target_dir)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

A rabbit hopped with config flair,
Found BetterGI.exe hanging there.
It copied nests with joyful speed,
And tests confirmed each little deed.
In CI winds, the bunnies cheer,
“Hop, run, and ship it” far and near.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and generally matches the main changes: unit tests and BetterGI/BGI config support.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch lvdown/ut

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai 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.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/ci.yml:
- Around line 29-30: The unittest discovery step is running from
OneDragon-ScriptChainer without the project top-level on the import path, so
root-level modules cannot be imported during CI. Update the workflow’s unittest
invocation to set the test top-level/import root explicitly for the discovery
command, using the existing unittest discovery step in the ci workflow so
imports resolve correctly when running tests from the repo root.

In `@copy_bettergi_config.py`:
- Around line 19-21: The script scanning loop in copy_bettergi_config.py assumes
every item in config.get("script_list", []) is a dict, so malformed entries can
raise AttributeError before the intended FileNotFoundError path. Update the loop
around script_path handling to defensively check each script entry’s type before
calling get, and skip or ignore any non-dict items so only valid dict entries
are inspected for "bettergi.exe".
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 68b58faa-3881-4ae1-803d-2032297c0df1

📥 Commits

Reviewing files that changed from the base of the PR and between bc3734d and a36117b.

📒 Files selected for processing (5)
  • .github/workflows/ci.yml
  • copy_bettergi_config.py
  • tests/test_copy_bettergi_config.py
  • tests/test_utils.py
  • utils.py

Comment thread .github/workflows/ci.yml
Comment on lines +29 to +30
working-directory: OneDragon-ScriptChainer
run: uv run python -m unittest discover -s ../tests -v

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Fix unittest top-level path so root modules are importable.

Line 30 runs discovery from OneDragon-ScriptChainer without setting project top-level, which causes the ModuleNotFoundError failures in CI.

Proposed fix
-        run: uv run python -m unittest discover -s ../tests -v
+        run: uv run python -m unittest discover -s ../tests -t .. -v
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
working-directory: OneDragon-ScriptChainer
run: uv run python -m unittest discover -s ../tests -v
working-directory: OneDragon-ScriptChainer
run: uv run python -m unittest discover -s ../tests -t .. -v
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/ci.yml around lines 29 - 30, The unittest discovery step
is running from OneDragon-ScriptChainer without the project top-level on the
import path, so root-level modules cannot be imported during CI. Update the
workflow’s unittest invocation to set the test top-level/import root explicitly
for the discovery command, using the existing unittest discovery step in the ci
workflow so imports resolve correctly when running tests from the repo root.

Source: Pipeline failures

Comment thread copy_bettergi_config.py
Comment on lines +19 to +21
for script in config.get("script_list", []):
script_path = script.get("script_path", "")
if script_path and Path(script_path).name.lower() == "bettergi.exe":

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Handle non-dict items in script_list defensively.

Line 20 assumes every script_list entry is a dict; malformed YAML entries will crash with AttributeError instead of your intended FileNotFoundError.

Proposed fix
     for script in config.get("script_list", []):
+        if not isinstance(script, dict):
+            continue
         script_path = script.get("script_path", "")
         if script_path and Path(script_path).name.lower() == "bettergi.exe":
             return Path(script_path)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
for script in config.get("script_list", []):
script_path = script.get("script_path", "")
if script_path and Path(script_path).name.lower() == "bettergi.exe":
for script in config.get("script_list", []):
if not isinstance(script, dict):
continue
script_path = script.get("script_path", "")
if script_path and Path(script_path).name.lower() == "bettergi.exe":
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@copy_bettergi_config.py` around lines 19 - 21, The script scanning loop in
copy_bettergi_config.py assumes every item in config.get("script_list", []) is a
dict, so malformed entries can raise AttributeError before the intended
FileNotFoundError path. Update the loop around script_path handling to
defensively check each script entry’s type before calling get, and skip or
ignore any non-dict items so only valid dict entries are inspected for
"bettergi.exe".

@LevelDownRefine
LevelDownRefine deleted the lvdown/ut branch June 25, 2026 10:08
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.

1 participant