Skip to content

fix: Resolve quickstart script failures and add automated testing #548

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Oct 29, 2025
Merged

fix: Resolve quickstart script failures and add automated testing #548

merged 1 commit into from
Oct 29, 2025
+251 −80

Conversation

@yehudit1987
Copy link
Contributor

@yehudit1987 yehudit1987 commented Oct 28, 2025

The quickstart script failed to execute successfully in local environment.
After fixing local issues, added CI testing which revealed additional
environment-specific failures. This commit includes all fixes plus
comprehensive testing infrastructure to prevent regressions.

Local Environment Fixes:

  • Fix critical services health check logic
  • Add proper error handling and timeout protection
  • Add helper functions for consistent messaging (success_msg, error_msg, section_header)
  • Fix duplicate YAML keys in config.yaml causing parsing failures
  • Improve service readiness detection

CI Environment Fixes:

  • Add CI detection to skip terminal-only operations (clear, browser prompts)
  • Optimize for CI constraints (disk cleanup, CI_MINIMAL_MODELS)

Testing Infrastructure:

  • Add quickstart-integration-test.yml workflow for E2E validation
  • Test actual routing functionality with real queries (not just health checks)
  • Add shell-syntax-check Makefile target
  • Integrate shell syntax validation into pre-commit workflow

Tested in both local and CI environments. Workflow passing successfully.
https://github.com/yehudit1987/semantic-router/actions/runs/18870299710/job/53846820124

Fixed issue: #547

@netlify
Copy link

netlify bot commented Oct 28, 2025 •
edited
Loading

✅ Deploy Preview for vllm-semantic-router ready!

Name Link
🔨 Latest commit 52faee3
🔍 Latest deploy log https://app.netlify.com/projects/vllm-semantic-router/deploys/69025eed2b4fa50008ee5d11
😎 Deploy Preview https://deploy-preview-548--vllm-semantic-router.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@github-actions
Copy link

github-actions bot commented Oct 28, 2025 •
edited
Loading

👥 vLLM Semantic Team Notification

The following members have been identified for the changed files in this PR and have been automatically assigned:

📁 Root Directory

Owners: @rootfs, @Xunzhuo
Files changed:

  • .github/workflows/quickstart-integration-test.yml
  • scripts/quickstart.sh

📁 config

Owners: @rootfs
Files changed:

  • config/config.yaml

📁 deploy

Owners: @rootfs, @Xunzhuo
Files changed:

  • deploy/docker-compose/docker-compose.yml

📁 tools

Owners: @yuluo-yx, @rootfs, @Xunzhuo
Files changed:

  • tools/make/linter.mk
vLLM

🎉 Thanks for your contributions!

This comment was automatically generated based on the OWNER files in the repository.

@yehudit1987 yehudit1987 marked this pull request as ready for review October 28, 2025 13:12
@rootfs
Copy link
Collaborator

rootfs commented Oct 28, 2025

@JaredforReal @yuluo-yx PTAL, thanks

@JaredforReal
Copy link
Collaborator

JaredforReal commented Oct 28, 2025

Left a few comments, PTAL @yehudit1987 Thanks!

yuluo-yx
yuluo-yx reviewed Oct 28, 2025
View reviewed changes
JaredforReal
JaredforReal reviewed Oct 28, 2025
View reviewed changes
@JaredforReal
Copy link
Collaborator

JaredforReal commented Oct 28, 2025

@yehudit1987 sorry my bad

yuluo-yx
yuluo-yx reviewed Oct 28, 2025
View reviewed changes
yuluo-yx
yuluo-yx reviewed Oct 28, 2025
View reviewed changes
@yehudit1987 yehudit1987 marked this pull request as draft October 29, 2025 12:39
@yehudit1987 yehudit1987 force-pushed the fix_quickstart_script branch 3 times, most recently from ac03f68 to 7c14a6c Compare October 29, 2025 13:25
@rootfs
Copy link
Collaborator

rootfs commented Oct 29, 2025

the quickstart failure was due to the GLIBC dependency, retesting with latest main

@yehudit1987 yehudit1987 marked this pull request as ready for review October 29, 2025 14:05
JaredforReal
JaredforReal suggested changes Oct 29, 2025
View reviewed changes
@JaredforReal
Copy link
Collaborator

JaredforReal commented Oct 29, 2025

@yehudit1987 Take another look, thanks! And others look good to me.

@yehudit1987 yehudit1987 force-pushed the fix_quickstart_script branch from f900752 to 61d167e Compare October 29, 2025 18:36
feat: fix quickstart.sh script
52faee3 
Signed-off-by: Yehudit Kerido <[email protected]>
@yehudit1987 yehudit1987 force-pushed the fix_quickstart_script branch from 61d167e to 52faee3 Compare October 29, 2025 18:37
@rootfs rootfs merged commit 7fddb43 into vllm-project:main Oct 29, 2025
10 checks passed
@yehudit1987 yehudit1987 deleted the fix_quickstart_script branch October 29, 2025 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rootfs rootfs Awaiting requested review from rootfs rootfs is a code owner

@Xunzhuo Xunzhuo Awaiting requested review from Xunzhuo Xunzhuo is a code owner

2 more reviewers

@yuluo-yx yuluo-yx yuluo-yx left review comments

@JaredforReal JaredforReal JaredforReal requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

@rootfs rootfs

@Xunzhuo Xunzhuo

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@yehudit1987 @rootfs @JaredforReal @yuluo-yx @Xunzhuo