[TST] Add v2 API

[tazarov]

Summary

This PR adds comprehensive testing infrastructure for the v2 API implementation, including TestContainers-based integration tests and multiple CI/CD workflows.

Changes

Testing

• ServerClientTest: Comprehensive test suite for v2 API operations
  • Basic CRUD operations (create, read, update, delete)
  • Collection management
  • Query and filtering capabilities
  • Batch operations and large dataset handling
  • Fluent API testing
  • Stress testing with 10,000+ records

CI/CD Workflows

1. v2-api-tests.yml: Main testing workflow

   • Matrix testing with ChromaDB versions (0.5.15, 0.5.20, latest)
   • Java versions 11 and 17
   • Automatic test reporting

2. v2-api-nightly.yml: Comprehensive nightly testing

   • Extended matrix with ChromaDB 0.4.24 through latest
   • All Java versions (8, 11, 17, 21)
   • Stress and performance tests
   • Automatic issue creation on failures

3. v2-api-pr-validation.yml: Quick PR validation

   • Fast feedback for pull requests
   • Code quality checks
   • Basic integration testing

4. v2-api-release.yml: Release validation

   • Full compatibility testing
   • Backward compatibility checks
   • Release artifact generation

Test Coverage

The test suite covers:

• ✅ Collection CRUD operations
• ✅ Document/embedding operations (add, update, upsert, delete)
• ✅ Query operations with filters
• ✅ Batch operations
• ✅ Complex where clauses
• ✅ Large dataset handling (100-10,000 records)
• ✅ Concurrent operations
• ✅ Fluent builder patterns

Testing Strategy

• Unit Tests: Fast, isolated tests for individual components
• Integration Tests: TestContainers-based tests with real ChromaDB
• Matrix Testing: Multiple ChromaDB and Java version combinations
• Performance Tests: Stress testing with large datasets

How to Test Locally

# Run all v2 API tests
mvn test -Dtest="tech.amikos.chromadb.v2.**"

# Run with specific ChromaDB version
export CHROMA_VERSION=0.5.15
mvn test -Dtest=ServerClientTest

Checklist

• Tests pass locally (with ChromaDB container)
• CI workflows are properly configured
• Test coverage is comprehensive
• Documentation is included in workflows
• Follows project conventions

Related Issues

• Part of v2 API implementation effort

Notes

• Tests require Docker for TestContainers
• CI will automatically test multiple ChromaDB versions
• Nightly tests provide extended coverage

[claude]

Code Review for PR #80

Overall, this is a well-structured PR that adds comprehensive testing infrastructure for the v2 API. The test coverage is extensive and the CI/CD workflows are thoughtfully designed.

✅ Strengths

• Excellent test coverage: Tests cover CRUD operations, batch processing, stress testing (10K+ records), concurrent operations, and various query scenarios
• Comprehensive CI matrix: Testing across multiple ChromaDB versions (0.4.24-latest) and Java versions (8, 11, 17, 21)
• Smart workflow design: Separate workflows for PR validation (fast), nightly (comprehensive), and release
• Good test isolation: Each test creates unique collections with UUIDs to prevent conflicts

🔧 Suggestions for Improvement

1. Resource Management: The stress test creates 10,000 records but lacks explicit cleanup. Consider adding:

   • Explicit collection deletion in @after methods
   • Connection pooling limits for concurrent tests
   • Memory usage monitoring for large-scale tests

2. Test Reliability:

   • Add retry logic for flaky container startup (currently waits 60x2s)
   • Consider using @rule for TestContainers lifecycle management
   • Add timeout annotations for long-running tests

3. Error Handling: The concurrent operations test (100 threads) should:

   • Assert specific success/failure counts
   • Include error message validation
   • Test connection pool exhaustion scenarios

4. Performance Assertions: Stress tests should include:

   • Response time thresholds
   • Memory usage limits
   • Query performance benchmarks

5. Minor Issues:

   • Missing assertion in testCountCollections() - should verify count increased by 1
   • Consider parameterized tests for similar test cases with different inputs
   • Add negative test cases (malformed data, exceeding limits)

🔒 Security Considerations

• Tests appear to use default/no authentication - consider adding auth test scenarios
• Environment variable handling looks secure

📊 Performance

• Batch size of 100 for 10K records is reasonable
• Consider adding configurable batch sizes via environment variables
• Thread pool size (10) in concurrent tests is conservative - good for stability

The PR significantly improves test coverage and CI/CD capabilities. With the suggested improvements, this will provide excellent quality assurance for the v2 API implementation.

[github-actions]

V2 API PR Validation Results

❌ Some checks failed

Changes Detected

• Source files changed:
• Test files changed:

---

🔗 View full workflow run

[graphite-app]

The shell wildcard pattern inside double quotes won't expand as expected. For reliable file detection, consider using:

if ls test-artifacts/stress-test-results-v2/TEST-*V2StressTest.xml &>/dev/null; then

This ensures proper glob expansion and redirects both stdout and stderr to avoid unnecessary output.

Suggested change

	if [ -f "test-artifacts/stress-test-results-v2/TEST-*V2StressTest.xml" ]; then
	if ls test-artifacts/stress-test-results-v2/TEST-*V2StressTest.xml &>/dev/null; then

Spotted by Diamond



Is this helpful? React 👍 or 👎 to let us know.

[graphite-app]

The workflow references steps.test-results.outputs.summary in the PR comment generation section, but there's no corresponding step with ID test-results that sets this output variable. This will cause the comment generation to use an empty value. Either:

1. Add a step with ID test-results that sets the summary output, or
2. Remove this reference and use another method to include test results in the PR comment

This issue appears in the v2-api-pr-validation.yml workflow file.

Suggested change

	const testResults = `${{ steps.test-results.outputs.summary }}`;
	const testResults = `No test results available`;

Spotted by Diamond



Is this helpful? React 👍 or 👎 to let us know.

[graphite-app]

There appears to be a job dependency issue in the workflow. The variable steps.version.outputs.version is defined in the validate-release job but is being referenced directly in the publish-release job. Since job outputs aren't automatically shared between jobs, this will cause the asset path and name to be undefined.

To fix this, either:

1. Pass the version as a job output from validate-release and reference it as needs.validate-release.outputs.version in the publish-release job, or
2. Re-extract the version in the publish-release job using the same logic

This ensures the artifact paths will be correctly resolved during the release process.

Suggested change

	asset_path: ./release/chromadb-java-client-${{ steps.version.outputs.version }}.jar
	asset_name: chromadb-java-client-${{ steps.version.outputs.version }}.jar
	asset_path: ./release/chromadb-java-client-${{ needs.validate-release.outputs.version }}.jar
	asset_name: chromadb-java-client-${{ needs.validate-release.outputs.version }}.jar

Spotted by Diamond



Is this helpful? React 👍 or 👎 to let us know.

[graphite-app]

The bash arithmetic syntax here is incorrect. When referencing GitHub Actions outputs in a bash arithmetic context, proper variable expansion is needed. Either of these approaches would work:

PREV_MINOR=$((${steps.version.outputs.minor} - 1))

Or with a fallback to prevent errors if the variable is unset:

PREV_MINOR=$((${steps.version.outputs.minor:-0} - 1))

This ensures the arithmetic operation is performed correctly with the value from the GitHub Actions output.

Suggested change

	PREV_MINOR=$((steps.version.outputs.minor - 1))
	PREV_MINOR=$((${steps.version.outputs.minor:-0} - 1))

Spotted by Diamond



Is this helpful? React 👍 or 👎 to let us know.

[claude]

Additional Code Review

Building on the previous review, here are some additional observations:

Test Quality Improvements

1. TestContainer Management: The static container setup could cause issues in parallel test execution. Consider using @ClassRule for better lifecycle management.

2. Missing Assertions: Several tests lack comprehensive assertions:

   • testListCollections() only checks size, not content
   • testQueryWithWhereClause() doesn't validate filtered results match the where clause
   • No validation that metadata is preserved after operations

3. Edge Cases: Add tests for:

   • Empty collections (query/delete on empty)
   • Duplicate IDs handling
   • Maximum embedding dimensions (current tests use 384, but what's the limit?)
   • Invalid UTF-8 in metadata/documents

Performance & Reliability

4. Workflow Optimization:

   • The nightly workflow matrix creates 28 jobs (7 versions × 4 Java versions) - consider using fail-fast: false strategically
   • Cache Docker images between matrix runs to reduce pull time

5. Flaky Test Prevention:

   • Replace Thread.sleep() with proper wait conditions
   • Use Awaitility library for async assertions
   • Add container readiness checks beyond simple HTTP health

CI/CD Enhancements

6. Test Reporting: The workflows generate reports but don't aggregate them. Consider:

   • Using test-reporter action for unified reporting
   • Publishing coverage to Codecov/Coveralls
   • Adding badges to README

7. Resource Limits: Add explicit resource constraints to prevent OOM in CI:
   MAVEN_OPTS: -Xmx2048m -XX:MaxMetaspaceSize=512m

Positive Notes

• Excellent use of builder pattern in tests
• Good separation of stress tests from functional tests
• Smart version matrix testing strategy

The test infrastructure is solid and will significantly improve project quality. The suggested improvements would make it production-grade.

[claude]

Code Review - Additional Observations

After reviewing the test implementation and CI workflows in detail, here are specific actionable improvements:

🐛 Critical Issues

1. TestContainer Lifecycle: The static container in ServerClientTest risks port conflicts and resource leaks. Use @ClassRule with .withReuse(true) for proper lifecycle management.

2. Incomplete Test Assertions:

   • testCountCollections(): Missing assertion that count increased
   • testQueryWithWhereClause(): No validation that results match filter criteria
   • testUpdateCollection(): Doesn't verify metadata was actually updated

⚡ Performance & Reliability

3. Workflow Resource Management:

   • Current MAVEN_OPTS uses 4GB heap - excessive for CI runners
   • Recommend: -Xmx2048m -XX:MaxMetaspaceSize=512m
   • Add container memory limits in docker run commands

4. Flaky Test Mitigation:

   • Replace TimeUnit.SECONDS.sleep(5) with proper readiness checks
   • Use TestContainers' built-in wait strategies like Wait.forHttp("/api/v1").forStatusCode(200)

🔍 Missing Test Coverage

5. Error Scenarios: Add tests for:

   • Connection failures/timeouts
   • Malformed embedding dimensions
   • Collection name conflicts
   • Null/empty parameters
   • Rate limiting behavior

6. Boundary Conditions:

   • Maximum collection name length
   • Maximum metadata size
   • Maximum batch size limits
   • Query result pagination

💡 Workflow Improvements

7. PR Validation Optimization:

   • Service containers usage is good but add startup verification
   • Consider caching test dependencies between runs
   • Add concurrency group to prevent parallel PR runs

8. Nightly Test Reporting:

   • Issue creation on failure is good
   • Add test result aggregation across matrix
   • Consider Slack/email notifications for failures

✅ What Works Well

• Comprehensive matrix testing strategy
• Good use of TestContainers
• Clear separation of test types
• Fluent API testing coverage

The foundation is strong - these improvements would elevate it to production quality. Great work on the comprehensive test coverage!

[claude]

Code Review for PR #80

This PR adds excellent test coverage and CI infrastructure for the v2 API. Here is my focused feedback:

✅ Strengths

• Comprehensive test matrix (ChromaDB 0.4.24-latest × Java 8-21)
• Well-structured CI workflows (PR/nightly/release)
• Good stress testing with 10K records and concurrent operations
• Clean builder pattern implementation

🔧 Key Improvements Needed

1. TestContainer Management
Static container setup in tests risks port conflicts. Use @ClassRule with .withReuse(true) for better lifecycle management and add proper wait strategies.

2. Missing Test Assertions

• testCountCollections(): Verify count actually increased
• testQueryWithWhereClause(): Validate results match filter criteria
• testUpdateCollection(): Confirm metadata was updated

3. Resource Management

• Current MAVEN_OPTS uses 4GB heap - excessive for CI
• Recommend: -Xmx2048m -XX:MaxMetaspaceSize=512m
• Add explicit cleanup in @after methods

4. Error Handling Tests
Add negative test cases:

• Malformed embeddings
• Duplicate IDs
• Connection timeouts
• Collection name conflicts

5. Flaky Test Prevention
Replace Thread.sleep() with proper wait conditions using Awaitility or TestContainers wait strategies.

📊 Performance

• Batch size of 100 is reasonable
• Thread pool size of 10 is conservative (good for CI stability)
• Consider parameterizing batch sizes via env vars

🔒 Security

Consider adding authentication test scenarios beyond the current no-auth tests.

The test infrastructure provides solid coverage. With these improvements, it will be production-grade.

[claude]

Code Review for V2 API Tests & CI Workflows

✅ Strengths

• Comprehensive test coverage: Good matrix testing across multiple ChromaDB versions (0.4.24-latest) and Java versions (8-21)
• Well-structured CI/CD: Separate workflows for nightly, PR validation, and release testing
• Stress testing: 10K+ record tests and concurrent operations validation
• TestContainers integration: Proper use for integration testing

🔧 Issues to Address

1. Remove unwanted files:

   • .DS_Store, out.json, oai-ollama.http, v2-notes, v2-openapi.json should not be committed
   • CollectionInterface.java.bak backup file should be removed
   • Add these to .gitignore

2. Package structure inconsistency:

   • CLAUDE.md specifies flat package structure in tech.amikos.chromadb.v2
   • Code references tech.amikos.chromadb.v2.http.HttpClient (sub-package)
   • Should follow the radical simplicity principle with no sub-packages

3. Test improvements needed:

   • ChromaClientTest uses version 1.1.0 which doesn't exist (latest is 0.5.x)
   • Missing error handling tests for network failures/timeouts
   • No tests for authentication providers (Basic, Token, ChromaToken)

4. CI workflow concerns:

   • Nightly workflow creates 368-line stress test inline - should use existing test files
   • PR validation compiles Java in workflow - should use Maven for consistency
   • Missing caching for Docker images could slow CI

5. Documentation files:

   • Multiple V2 documentation files (V2_API_EXAMPLE.md, V2_API_REFACTORED.md, etc.) - consider consolidating
   • Follow project convention of not creating docs unless requested

🚀 Recommendations

1. Clean up repository files and update .gitignore
2. Fix package structure to match CLAUDE.md specification
3. Use realistic ChromaDB versions in tests
4. Move inline test code to proper test classes
5. Add tests for auth providers and error scenarios

Overall, solid testing infrastructure that needs cleanup and alignment with project conventions.

[claude]

Focused Code Review

🚨 Priority Issues Not Previously Mentioned

1. ChromaDB Version Mismatch: Tests use version 1.1.0 as default but ChromaDB latest stable is 0.5.x. This will cause immediate test failures.

2. Uncommitted Files: Several files should not be in version control:

   • .DS_Store (macOS metadata)
   • out.json (appears to be debug output)
   • CollectionInterface.java.bak (backup file)
   • oai-ollama.http, v2-notes, v2-openapi.json (temporary/debug files)

3. Package Structure Violation: Code violates CLAUDE.md flat package principle - HttpClient references suggest sub-packages exist.

4. Workflow Redundancy: v2-api-nightly.yml duplicates test code inline rather than using existing test classes.

✅ What Works Well

• TestContainers setup is solid
• Matrix testing strategy is comprehensive
• Fluent API test coverage is thorough

🎯 Quick Wins

1. Update default ChromaDB version to 0.5.20 or latest
2. Add unwanted files to .gitignore
3. Consolidate duplicate V2 documentation files
4. Use existing test classes in workflows instead of inline code

The test infrastructure is valuable - these fixes will make it merge-ready.

[claude]

Code Review: V2 API Tests and CI Workflows

Strengths

• Comprehensive test coverage with CRUD operations, batch processing, stress testing, and fluent API testing
• Robust CI/CD strategy with four well-designed workflows
• Matrix testing across ChromaDB versions 1.0.0-1.1.0+ and Java versions 8, 11, 17, 21
• Proper TestContainers integration for real ChromaDB instances
• Good error handling with cleanup in @after methods

Issues to Address

1. .DS_Store file should not be committed - add to .gitignore

2. ChromaDB v2 API compatibility concern: workflows reference ChromaDB 1.x but tests use /api/v2/heartbeat endpoints that may not exist yet

3. Hardcoded Thread.sleep() calls could cause flaky tests - use TestContainers Wait strategies

4. Multiple documentation files (V2_API_EXAMPLE.md, V2_API_REFACTORED.md, etc.) should be consolidated

5. Nightly workflow creates V2StressTest.java inline - should be a proper test file

Security

• No hardcoded credentials found
• Environment variables properly used
• Auth providers well abstracted

Test Coverage Gaps

• Missing negative test cases for auth failures and network errors
• No tests for collection metadata operations
• Missing embedding function configuration tests

Recommendations

1. Add retry logic for container startup
2. Use proper logging instead of System.out.println
3. Add @category annotations for test types
4. Consider WireMock for unit tests

Overall solid foundation. Main concern is ChromaDB version compatibility with v2 API endpoints.

[graphite-app]

There's a mismatch between the Java versions in the test matrix and the compatibility report. The matrix in v2-api-tests.yml specifies Java versions [8, 17], but the compatibility report in the same file (lines 208-209) checks for Java 11 and 17. This will cause the compatibility report to show incorrect results for Java 11, which isn't actually being tested. Consider either adding Java 11 to the test matrix or updating the compatibility report to check for Java 8 instead.

Suggested change

	java-version: [8, 17]
	java-version: [8, 11, 17]

Spotted by Diamond



Is this helpful? React 👍 or 👎 to let us know.