Skip to content

Conversation

@szedan-rh
Copy link
Contributor

Summary

This PR implements comprehensive end-to-end tests for keyword routing functionality, addressing Issue #667.

Test Coverage

35 Tests Covering:

  • OR operator (6 tests): match when any keyword is present
  • AND operator (5 tests): match when all keywords must be present
  • NOR operator (2 tests): match when no keywords are present
  • Case sensitivity (3 tests): case-sensitive and case-insensitive matching
  • Word boundaries (3 tests): prevent partial word matches
  • Regex special characters (3 tests): handle dots, asterisks literally
  • Edge cases (8 tests): empty text, Unicode, emoji, newlines
  • Multiple rule matching (2 tests): priority order validation
  • Confidence scores (1 test): keyword matches return 1.0
  • JSON test data (1 test): load and validate test cases
  • Error handling (2 tests): invalid operators, empty keywords

Test Results

Ran 35 of 35 Specs in 0.012 seconds
✅ 35 Passed | ❌ 0 Failed | ⏭️ 0 Pending | ⏩ 0 Skipped
PASS

Performance:

  • Total: 35 tests
  • Average test time: < 0.3ms per test
  • Race detection: PASS (no race conditions)

Files Added

  • e2e-tests/testcases/suite_test.go: Ginkgo test suite entry point
  • e2e-tests/testcases/keyword_routing_test.go: 35 comprehensive tests
  • e2e-tests/testcases/helpers.go: Test helper functions
  • e2e-tests/testcases/testdata/keyword_routing_cases.json: 26 JSON test cases
  • e2e-tests/testcases/go.mod & go.sum: Module configuration
  • .github/workflows/unit-test-e2e-testcases.yml: CI/CD workflow

Technical Highlights

NOR Operator Isolation

Created dedicated classifier for NOR tests to prevent false matches in tests expecting empty results.

Test Framework

  • Uses Ginkgo v2 + Gomega for BDD-style testing
  • Table-driven tests for similar test cases
  • JSON test data for easy maintenance
  • Proper module setup with replace directives

CI Integration

GitHub Actions workflow includes:

  • Keyword routing tests
  • Race condition detection
  • Code linting
  • Coverage reporting to Codecov

Checklist

  • All tests pass locally
  • No race conditions detected
  • Code passes linting (go fmt applied)
  • Test data in JSON format
  • Helper functions documented
  • CI workflow configured

Related Issues

Fixes #667


Signed-off-by: szedan

@netlify
Copy link

netlify bot commented Nov 17, 2025

Deploy Preview for vllm-semantic-router ready!

Name Link
🔨 Latest commit 7672d1f
🔍 Latest deploy log https://app.netlify.com/projects/vllm-semantic-router/deploys/691caf7423cecb0008e16d29
😎 Deploy Preview https://deploy-preview-684--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.

@szedan-rh szedan-rh marked this pull request as draft November 17, 2025 15:31
@codecov-commenter
Copy link

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

@szedan-rh szedan-rh force-pushed the test_keyword_routing branch from b0dfb6a to 7d27e11 Compare November 17, 2025 17:15
@szedan-rh szedan-rh marked this pull request as ready for review November 18, 2025 12:42
@szedan-rh
Copy link
Contributor Author

szedan-rh commented Nov 18, 2025

Hi @Xunzhuo ,
This is the first PR for test keyword routing, could you please review?
Another PRs for the next tests will be raised separately

@rootfs
Copy link
Collaborator

rootfs commented Nov 18, 2025

@srini-abhiram PTAL, thanks

@github-actions
Copy link

github-actions bot commented Nov 18, 2025

👥 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/unit-test-e2e-testcases.yml

📁 e2e-tests

Owners: @yossiovadia
Files changed:

  • e2e-tests/testcases/go.mod
  • e2e-tests/testcases/go.sum
  • e2e-tests/testcases/helpers.go
  • e2e-tests/testcases/keyword_routing_test.go
  • e2e-tests/testcases/suite_test.go
  • e2e-tests/testcases/testdata/keyword_routing_cases.json

vLLM

🎉 Thanks for your contributions!

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

@szedan-rh szedan-rh force-pushed the test_keyword_routing branch from e1a1311 to 6a09f09 Compare November 18, 2025 17:27
Implement comprehensive end-to-end tests for keyword routing functionality
to address Issue vllm-project#667.

Test Coverage:
- OR operator: any keyword matches
- AND operator: all keywords must match
- NOR operator: no keywords match
- Case-sensitive vs case-insensitive matching
- Regex pattern matching and special character handling
- Word boundary detection
- Edge cases: empty text, Unicode, emoji, punctuation
- Multiple rule matching and priority
- Confidence score validation
- Error handling

Implementation Details:
- 35 comprehensive tests using Ginkgo v2 and Gomega
- JSON test data files for maintainability (26 test cases)
- Helper functions for test setup
- Isolated NOR operator tests to prevent false matches
- CI/CD integration with GitHub Actions workflow
- Coverage measurement: 87.1% of keyword_classifier.go (exceeds 80% threshold)
- Race condition detection
- golangci-lint integration

Technical Fixes:
- Removed unused helper functions to pass linter
- Disabled CUDA features for CI environment (--no-default-features)
- Fixed coverage reporting to measure keyword_classifier.go specifically
- Removed -run filter that doesn't work with Ginkgo tests

Files Added:
- .github/workflows/unit-test-e2e-testcases.yml
- e2e-tests/testcases/suite_test.go
- e2e-tests/testcases/keyword_routing_test.go
- e2e-tests/testcases/helpers.go
- e2e-tests/testcases/testdata/keyword_routing_cases.json
- e2e-tests/testcases/go.mod
- e2e-tests/testcases/go.sum

Signed-off-by: Senan Zedan <[email protected]>
@szedan-rh szedan-rh force-pushed the test_keyword_routing branch from 6a09f09 to 7672d1f Compare November 18, 2025 17:40
@szedan-rh
Copy link
Contributor Author

After sync the branch - fix back all the failing unit tests and squash all commits to one commit for easy code review.

@rootfs rootfs merged commit 1c8c932 into vllm-project:main Nov 18, 2025
24 checks passed
@Xunzhuo
Copy link
Member

Xunzhuo commented Nov 19, 2025

@szedan-rh i think the testcases is located incorrectly, plz move the cases into e2e/testcases, this will trigger the e2e tests in CI, plz read other testcases to understand how it works

@szedan-rh
Copy link
Contributor Author

You Right, I put them under e2e-tests.
will change that

@srini-abhiram
Copy link
Contributor

srini-abhiram commented Nov 19, 2025

There are some un unused types in helpers.go such as EmbeddingTestCase, HybridTestCase.
Also, the matched_keywords field in keyword_routing_cases.json is not asserted in the test cases, currently only the matched category is asserted. This might reproduce the incorrect matching bug, for example: star in stardom.

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.

[E2E Testing] Add comprehensive test coverage for routing strategies and filters

6 participants