feat(conformance): add TypeScript/Node.js SDK conformance driver (#1658)#1661
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughAdds a Node.js TypeScript SDK conformance runner and Python driver for ChangesTypeScript SDK conformance
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ConformanceSuite
participant TypeScriptSDKConformanceDriver
participant runner.mjs
participant FileOrganizerClient
participant FastAPIApp
ConformanceSuite->>TypeScriptSDKConformanceDriver: execute scan, preview, or execute
TypeScriptSDKConformanceDriver->>FastAPIApp: start local server
TypeScriptSDKConformanceDriver->>runner.mjs: pass action and request data
runner.mjs->>FileOrganizerClient: invoke SDK operation
FileOrganizerClient->>FastAPIApp: send HTTP request
FastAPIApp-->>FileOrganizerClient: return operation response
FileOrganizerClient-->>runner.mjs: return SDK result
runner.mjs-->>TypeScriptSDKConformanceDriver: emit JSON outcome
TypeScriptSDKConformanceDriver-->>ConformanceSuite: return conformance payload
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
fa1d746 to
48442c5
Compare
48442c5 to
013dbc8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/conformance/driver.py (2)
1253-1256: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winBusy-wait loop spins a CPU core while waiting for server startup.
Replacing
time.sleep()with a hotwhileloop avoids the sleep call but pegs a core pollingself._server.startedwith no yield. Consider a tiny sleep (e.g.time.sleep(0.005)) inside the loop, or better, athreading.Eventset from a startup callback, instead of a pure spin.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/conformance/driver.py` around lines 1253 - 1256, Update the server-startup wait loop around self._server.started to yield between polls, preferably by adding a short time.sleep delay while preserving the existing timeout and RuntimeError behavior. Avoid introducing broader synchronization changes unless an existing startup event or callback is already available.
1286-1286: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider bounding the Node subprocess with a timeout.
subprocess.runhas notimeout=. The TS client applies its own per-requestAbortControllertimeout, which reduces risk, but a hang in the Node process itself (unresolved promise, dangling handle) would still block the test run indefinitely. Addtimeout=and handlesubprocess.TimeoutExpiredfor a bounded, diagnosable failure instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/conformance/driver.py` at line 1286, Add a finite timeout to the subprocess.run call in the Node execution path, and catch subprocess.TimeoutExpired to report a clear, diagnosable failure while preserving the existing result handling for completed processes. Use the surrounding driver’s established timeout or failure-reporting conventions if available.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/conformance/driver.py`:
- Around line 1243-1259: Add a close() method to TypeScriptSDKConformanceDriver
that signals its uvicorn.Server to exit and joins the server thread, then update
the conformance fixture to use teardown and invoke close() for this driver
instead of returning it without cleanup. Preserve existing driver setup and
ensure cleanup runs after each parametrized test invocation.
---
Nitpick comments:
In `@tests/conformance/driver.py`:
- Around line 1253-1256: Update the server-startup wait loop around
self._server.started to yield between polls, preferably by adding a short
time.sleep delay while preserving the existing timeout and RuntimeError
behavior. Avoid introducing broader synchronization changes unless an existing
startup event or callback is already available.
- Line 1286: Add a finite timeout to the subprocess.run call in the Node
execution path, and catch subprocess.TimeoutExpired to report a clear,
diagnosable failure while preserving the existing result handling for completed
processes. Use the surrounding driver’s established timeout or failure-reporting
conventions if available.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a0f62654-9087-457f-8c2e-7bd3cddc8e1d
📒 Files selected for processing (7)
docs/developer/capability-matrix.mdsrc/file_organizer/core/capability_registry.jsontests/conformance/conftest.pytests/conformance/driver.pytests/conformance/test_direct_service_conformance.pytests/conformance/ts_driver/runner.mjstests/core/test_capabilities.py
9bb6d67 to
220a54c
Compare
Summary
Builds a Node.js/TypeScript conformance driver for the
TYPESCRIPT_SDKsurface exercisingorganization.scan,organization.preview, andorganization.executeagainst a real local backend environment.tests/conformance/ts_driver/runner.mjscallingFileOrganizerClientfromsrc/file_organizer/client/typescript/client.ts.TypeScriptSDKConformanceDriverintests/conformance/driver.pyrunning an in-process ephemeraluvicornserver.TypeScriptSDKConformanceDriverintests/conformance/conftest.pyandtests/conformance/test_direct_service_conformance.py.capability_registry.jsonto setconformance_status: "verified"fortypescript-sdkonorganization.scan,organization.preview, andorganization.execute.tests/core/test_capabilities.pyand regeneratesdocs/developer/capability-matrix.md.python scripts/verify_claims_freshness.py.Review & Audit
runner.mjs, which immediately triggered 3 test failures, empirically verifying test suite defect detection.Closes #1658
Summary by CodeRabbit
New Features
Documentation
Tests