Skip to content

Conversation

DUDINGDDI
Copy link
Contributor

Purpose

Does this introduce a breaking change?

[ ] Yes
[x] No

Pull Request Type

What kind of change does this Pull Request introduce?

[ ] Bugfix
[x] New feature
[ ] Refactoring (no functional changes, no api changes)
[ ] Documentation content changes
[ ] Other... Please describe:

README updated?

The top-level readme for this repo contains a link to each sample in the repo. If you're adding a new sample did you update the readme?

[ ] Yes
[x] No
[ ] N/A

How to Test

  • Get the code
git clone https://github.com/DUDINGDDI/open-chat-playground.git
cd open-chat-playground
git checkout feature/Command-Line-Argument-Parsing-Docker-Model-Runner-277
  • Test the code
# Run all unit tests
dotnet test --filter "Category=UnitTest"

# Run only Docker Model Runner argument options tests
dotnet test --filter "Category=UnitTest&FullyQualifiedName~DockerModelRunnerArgumentOptionsTests"

# Build the application
dotnet restore && dotnet build

# Test CLI argument parsing with help
dotnet run --project src/OpenChat.PlaygroundApp -- --connector-type DockerModelRunner --help

# Test with CLI arguments (requires Docker Model Runner running)
dotnet run --project src/OpenChat.PlaygroundApp -- --connector-type DockerModelRunner --base-url http://localhost:12434 --model ai/smollm2

# Test with config file only
dotnet run --project src/OpenChat.PlaygroundApp -- --connector-type DockerModelRunner

# Test in Docker container
docker build -f Dockerfile -t openchat-playground:latest .
docker run -i --rm -p 8080:8080 openchat-playground:latest --connector-type DockerModelRunner --base-url http://host.docker.internal:12434 --model ai/phi-4

What to Check

Verify that the following are valid

  • Command-line argument parsing for --base-url and --model parameters
  • Configuration priority: CLI args > Config file (appsettings.json)
  • Help display shows Docker Model Runner options with defaults (base-url: http://localhost:12434, model: ai/smollm2)
  • ArgumentOptions.Parse() correctly handles DockerModelRunnerArgumentOptions case
  • All 18 unit tests pass for DockerModelRunnerArgumentOptionsTests
  • ArgumentOptionsTests includes DockerModelRunner in inheritance and connector type tests

Other Information

  • Added Docker Model Runner connector arguments to ArgumentOptions.cs arguments array (lines 31-32)
  • Implemented ParseOptions() method in DockerModelRunnerArgumentOptions.cs (lines 22-54)
  • Updated DisplayHelpForDockerModelRunner() with proper argument descriptions (lines 364-365)
  • Created comprehensive test suite following AnthropicArgumentOptionsTests pattern (293 lines)
  • Uncommented Docker Model Runner test cases in ArgumentOptionsTests.cs (3 locations)
  • All 462 unit tests pass successfully

@justinyoo
Copy link
Contributor

@DUDINGDDI 머지 컨플릭트 해결해 주세요.

@DUDINGDDI DUDINGDDI requested a review from justinyoo October 10, 2025 13:48
Copy link
Contributor

@justinyoo justinyoo left a comment

Choose a reason for hiding this comment

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

환경변수 관련 테스트는 이 태스크에서 다루지 않습니다. 다 빼고 커맨드라인 관련 해서만 테스트 코드 작성해 주세요.

Comment on lines +19 to +20
string? envBaseUrl = null,
string? envModel = null
Copy link
Contributor

Choose a reason for hiding this comment

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

예전과 같이 똑같은 실수를 하고 있습니다. 여기선 커맨드라인 아규먼트만 테스트 하니까 환경 변수 관련 테스트는 하지 않아요. 이와 관련한 내용은 다 빼세요.

@justinyoo
Copy link
Contributor

@DUDINGDDI 더이상 작업하실 수 없는 상황인 듯 하여 이 PR은 닫고 다른 분께 넘기겠습니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Command-Line Argument Parsing: Docker Model Runner

3 participants