Skip to content

test(onboard): cover Windows Ollama negative probe paths#4719

Merged
cv merged 2 commits into
NVIDIA:mainfrom
Thabhelo:test/onboard-windows-ollama-negative-paths
Jun 4, 2026
Merged

test(onboard): cover Windows Ollama negative probe paths#4719
cv merged 2 commits into
NVIDIA:mainfrom
Thabhelo:test/onboard-windows-ollama-negative-paths

Conversation

@Thabhelo
Copy link
Copy Markdown
Contributor

@Thabhelo Thabhelo commented Jun 3, 2026

Summary

Follow-up to #4089: add negative-path tests for detectWindowsHostOllama and clarify that installed reflects on-disk binary presence while onboard still gates Start/Restart on reachability (#3949).

Related Issue

Follow-up to #4089 (closed #4066).

Changes

  • Add tests for !isWsl() early return and all-probes-miss → installed: false
  • Document consumer-side Start/Restart gating in windows-host-ollama.ts

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Verification

  • npx prek run --all-files passes
  • npm test passes
  • Tests added or updated for new or changed behavior
  • No secrets, API keys, or credentials committed
  • Docs updated for user-facing behavior changes
  • make docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: Thabhelo 50872400+Thabhelo@users.noreply.github.com

Summary by CodeRabbit

  • Tests

    • Improved test coverage for Ollama detection on Windows.
  • Documentation

    • Added internal clarifications for detection behavior and system integration.

@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented Jun 3, 2026

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@Thabhelo
Copy link
Copy Markdown
Contributor Author

Thabhelo commented Jun 3, 2026

@prekshivyas Following up on the optional notes from #4089 / #4066 — added the negative-path tests (!isWsl and all probes miss) and a short comment on Start/Restart gating. Thanks for the review.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jun 3, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 3dc8431c-8d42-4984-bc0e-39dd6f81b165

📥 Commits

Reviewing files that changed from the base of the PR and between bb7e32b and 2c9f120.

📒 Files selected for processing (2)
  • src/lib/onboard/windows-host-ollama.test.ts
  • src/lib/onboard/windows-host-ollama.ts

📝 Walkthrough

Walkthrough

This PR adds test coverage for edge cases in Windows Ollama detection and clarifies the semantic difference between binary installation and daemon reachability that affects onboarding menu options.

Changes

Windows Ollama Detection Coverage

Layer / File(s) Summary
Test coverage for uninstalled states
src/lib/onboard/windows-host-ollama.test.ts
Two new test cases verify detectWindowsHostOllama() returns uninstalled state and omits probing when isWsl is false, and when all Windows probing commands return empty results.
Documentation clarifications
src/lib/onboard/windows-host-ollama.ts
Inline comments clarify that installed reflects on-disk binary presence (not daemon status) and that onboarding Start/Restart gating depends on reachability and loopback binding.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Suggested labels

area: cli, area: local-models, platform: windows, platform: wsl

Suggested reviewers

  • prekshivyas

Poem

🐰 A test for the silent, uninstalled state,
Where Ollama sleeps and binaries wait,
Comments now whisper which gates to release,
Onboarding flows with detective's peace!

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR adds tests for negative paths and clarifying documentation, but does not address the core fix proposed in #4066: moving the filesystem check before the PID guard. This PR is a follow-up addressing test coverage and documentation only. The root cause fix from #4066 (moving GET_KNOWN_OLLAMA_INSTALL_PATH before the PID guard) should be addressed in a separate PR.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding test cases for negative probe paths in Windows Ollama detection.
Out of Scope Changes check ✅ Passed All changes are in-scope: test cases for negative paths and inline documentation clarifying the installed flag behavior directly support the PR's stated follow-up objectives.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@cv cv added the v0.0.59 Release target label Jun 4, 2026
@cv cv merged commit d43077e into NVIDIA:main Jun 4, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v0.0.59 Release target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[WSL2][Onboard] nemoclaw onboard shows "Install Ollama on Windows host" instead of "Start" when Ollama binary exists but is not running and not in PATH

2 participants