-
Notifications
You must be signed in to change notification settings - Fork 5
feat: new testRun for transformations #4886
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.
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
base: develop
Are you sure you want to change the base?
Conversation
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Route src/routes/userTransform.ts |
Adds new POST route /transformation/testRun guarded by RouteActivationMiddleware.isUserTransformTestRouteActive. |
Controller src/controllers/userTransform.ts |
Adds UserTransformController.testRun(ctx) to extract request body (input, code, language, codeVersion, dependencies), build events/trRevCode/libraryVersionIDs, invoke UserTransformService.testTransformRoutine(...), set response and post-process. |
Types src/types/userTransformation.ts |
Introduces exported TestRunRequestBody and supporting types (Dependencies, LibraryInput, CredentialInput). |
Transformer (runtime behavior) src/util/customTransformer.js |
In testMode with codeVersion === '1', error entries in transformed events now include the original event metadata alongside the error (changes response shape for test path). |
Sequence Diagram(s)
sequenceDiagram
participant Client
participant Route
participant Controller as UserTransformController
participant Service as UserTransformService
participant Transformer as CustomTransformer
participant Response
Client->>Route: POST /transformation/testRun
activate Route
Route->>Route: RouteActivationMiddleware check
Route->>Controller: testRun(ctx)
deactivate Route
activate Controller
Controller->>Controller: Parse body (input, code, language,<br/>codeVersion, dependencies)
Controller->>Controller: Build events, trRevCode,<br/>libraryVersionIDs
Controller->>Service: testTransformRoutine(params)
deactivate Controller
activate Service
Service->>Transformer: Request transform of events with code
deactivate Service
activate Transformer
alt Error (codeVersion '1')
Transformer->>Transformer: Produce error entry including<br/>original event metadata + error
else Success
Transformer->>Transformer: Produce transformed event(s)
end
Transformer-->>Service: Transformed events / errors
deactivate Transformer
activate Service
Service-->>Controller: Return results
deactivate Service
activate Controller
Controller->>Controller: Set ctx.body, post-process, log
Controller-->>Response: Return response
deactivate Controller
Response-->>Client: Test run result
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested reviewers
- krishna2020
- Jayachand
- ItsSudip
- sivashanmukh
Pre-merge checks and finishing touches
❌ Failed checks (2 warnings)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Description check | The PR description covers the primary change but lacks critical details from the template such as Linear task ID, broader objectives explanation, and testing/documentation confirmation checkboxes. | Add the related Linear task ID (Resolves INT-XXX), expand on objectives section with dependent changes if any, and ensure developer and reviewer checklists are properly completed rather than left blank. | |
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | You can run @coderabbitai generate docstrings to improve docstring coverage. |
✅ Passed checks (1 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The PR title accurately describes the main change: introducing a new testRun endpoint for transformations, which aligns with the file changes and objectives. |
✨ Finishing touches
- 📝 Generate docstrings
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 @coderabbitai help to get the list of available commands and usage tips.
15e1c16 to
bf9810a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Fix all issues with AI Agents 🤖
In @src/controllers/userTransform.ts:
- Line 1: The file src/controllers/userTransform.ts has formatting/style issues;
run Prettier on it (e.g., execute `prettier --write
src/controllers/userTransform.ts`) or apply your project's Prettier config so
the import line and the rest of the file are consistently formatted (ensure the
existing import { Context } from 'koa'; and surrounding code adhere to Prettier
rules), then stage and commit the formatted file.
- Around line 127-139: The testRun method extracts request fields without
validation; add checks at the start of public static async testRun(ctx: Context)
to ensure required fields (at least code and dependencies, and that
dependencies.libraries is an array) are present and non-empty, and return a 400
error when missing (use the same error-handling pattern as testTransformLibrary
which validates code), e.g., validate trRevCode.code and dependencies before
computing libraryVersionIDs and throw/ctx.throw with a clear message if
validation fails.
🧹 Nitpick comments (1)
src/types/userTransformation.ts (1)
25-38: Consider exporting helper types for reusability.The
CredentialInput,LibraryInput, andDependenciestypes are not exported, limiting their reuse. If other modules might need to reference these types independently (e.g., for validation or utility functions), consider exporting them.Additionally, for consistency, you might standardize on using either
interfaceortypethroughout (currentlyLibraryInputis a type while the others are interfaces).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/controllers/userTransform.tssrc/routes/userTransform.tssrc/types/userTransformation.tssrc/util/customTransformer.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.js
⚙️ CodeRabbit configuration file
Focus on ESLint errors (max 3) and warnings (max 5).
Files:
src/util/customTransformer.js
🧠 Learnings (3)
📚 Learning: 2025-12-11T10:52:50.968Z
Learnt from: maheshkutty
Repo: rudderlabs/rudder-transformer PR: 4855
File: src/cdk/v2/destinations/zoho/utils.ts:251-322
Timestamp: 2025-12-11T10:52:50.968Z
Learning: In this repo, HTTP-wrapping utilities should not throw. If a function like handleHttpRequest wraps all HTTP calls (e.g., axios) in try/catch and always returns a structured object such as { httpResponse, processedResponse }, then callers must inspect response.status or response.success rather than using try/catch to handle errors. Apply this pattern to similar network utilities (e.g., src/cdk/v2/destinations/**/utils.ts) to ensure consistent error handling and avoid exception flow in favor of structured responses.
Applied to files:
src/routes/userTransform.tssrc/controllers/userTransform.tssrc/types/userTransformation.ts
📚 Learning: 2025-04-02T08:21:38.217Z
Learnt from: vinayteki95
Repo: rudderlabs/rudder-transformer PR: 4220
File: src/middlewares/routerTransformCompactedPayloadV1.ts:13-24
Timestamp: 2025-04-02T08:21:38.217Z
Learning: In the Rudder Transformer codebase, use generic Error objects instead of TransformationError when error monitoring with Bugsnag is needed, as TransformationError is configured to not trigger Bugsnag alerts.
Applied to files:
src/util/customTransformer.js
📚 Learning: 2025-05-29T13:29:39.436Z
Learnt from: maheshkutty
Repo: rudderlabs/rudder-transformer PR: 4359
File: src/v0/util/index.js:2272-2272
Timestamp: 2025-05-29T13:29:39.436Z
Learning: The `combineBatchRequestsWithSameJobIds` function in `src/v0/util/index.js` is used by both Mixpanel and Google AdWords Offline Conversions destinations in production. The function assumes metadata is always an array in multiple operations (forEach, filter, sort, map) and needs defensive programming to handle non-array metadata cases to prevent runtime errors.
Applied to files:
src/util/customTransformer.js
🧬 Code graph analysis (1)
src/routes/userTransform.ts (2)
src/middlewares/routeActivation.ts (1)
RouteActivationMiddleware(20-117)src/controllers/userTransform.ts (1)
UserTransformController(19-155)
🪛 GitHub Actions: Verify
src/controllers/userTransform.ts
[warning] 1-1: Code style issues found in the file. Run 'prettier --write' to fix.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Code Coverage
- GitHub Check: Build User Transformer Docker Image - PR / Build Transformer Docker Image ARM64
- GitHub Check: Build User Transformer Docker Image - PR / Build Transformer Docker Image AMD64
- GitHub Check: Build Transformer Docker Image - PR / Build Transformer Docker Image AMD64
- GitHub Check: Build Transformer Docker Image - PR / Build Transformer Docker Image ARM64
- GitHub Check: UT Tests
- GitHub Check: test_and_publish
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (6)
src/util/customTransformer.js (1)
359-369: LGTM! Improved error context in test mode.The addition of metadata to error responses in test mode enhances debugging capabilities by preserving the original event context alongside the error. This change aligns well with the new testRun endpoint's requirements.
src/types/userTransformation.ts (1)
40-46: LGTM! Type definitions are appropriate for the test endpoint.The
TestRunRequestBodyinterface provides the right balance of type safety and flexibility for testing transformations. The permissiveRecord<string, any>[]for input is appropriate given the test endpoint needs to accept various event structures.src/routes/userTransform.ts (1)
27-31: LGTM! Route definition follows established patterns.The new
/transformation/testRunroute is properly configured with appropriate middleware and follows the same pattern as the existing test endpoints. The route activation middleware ensures the endpoint is only accessible when the feature is enabled.src/controllers/userTransform.ts (3)
4-8: LGTM! Import statement properly updated.The addition of
TestRunRequestBodyto the imports is clean and appropriately grouped with related types.
141-146: LGTM! Service call appropriately reuses existing test infrastructure.The method correctly delegates to
UserTransformService.testTransformRoutine, reusing the established test transformation pipeline.
147-148: LGTM! Response handling follows established patterns.The response handling is consistent with the existing
testTransformmethod.
| @@ -1,7 +1,11 @@ | |||
| import { Context } from 'koa'; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix code formatting with Prettier.
The pipeline detected code style issues in this file. Please run prettier --write src/controllers/userTransform.ts to ensure consistent formatting across the codebase.
#!/bin/bash
# Format the file with Prettier
prettier --write src/controllers/userTransform.ts🧰 Tools
🪛 GitHub Actions: Verify
[warning] 1-1: Code style issues found in the file. Run 'prettier --write' to fix.
🤖 Prompt for AI Agents
In @src/controllers/userTransform.ts around line 1, The file
src/controllers/userTransform.ts has formatting/style issues; run Prettier on it
(e.g., execute `prettier --write src/controllers/userTransform.ts`) or apply
your project's Prettier config so the import line and the rest of the file are
consistently formatted (ensure the existing import { Context } from 'koa'; and
surrounding code adhere to Prettier rules), then stage and commit the formatted
file.
|
Allure Test reports for this run are available at:
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #4886 +/- ##
===========================================
- Coverage 92.25% 92.22% -0.04%
===========================================
Files 654 654
Lines 35358 35409 +51
Branches 8315 8337 +22
===========================================
+ Hits 32620 32656 +36
- Misses 2503 2518 +15
Partials 235 235 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
bf9810a to
5090a06
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/routes/userTransform.ts (1)
27-31: Route implementation is correct and consistent with existing test endpoints.The
/transformation/testRunroute is properly structured with appropriate middleware and theUserTransformController.testRunmethod is implemented correctly.Since the PR objectives indicate this endpoint is intended to become the primary transformation testing endpoint, consider adding
StatsMiddleware.executionStatsfor observability and monitoring, which aligns with the production/customTransformroute pattern. This would provide better visibility as usage grows.Optional: Add stats middleware for observability
router.post( '/transformation/testRun', RouteActivationMiddleware.isUserTransformTestRouteActive, + StatsMiddleware.executionStats, UserTransformController.testRun, );
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/controllers/userTransform.tssrc/routes/userTransform.tssrc/types/userTransformation.tssrc/util/customTransformer.js
🚧 Files skipped from review as they are similar to previous changes (3)
- src/controllers/userTransform.ts
- src/util/customTransformer.js
- src/types/userTransformation.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-11T10:52:50.968Z
Learnt from: maheshkutty
Repo: rudderlabs/rudder-transformer PR: 4855
File: src/cdk/v2/destinations/zoho/utils.ts:251-322
Timestamp: 2025-12-11T10:52:50.968Z
Learning: In this repo, HTTP-wrapping utilities should not throw. If a function like handleHttpRequest wraps all HTTP calls (e.g., axios) in try/catch and always returns a structured object such as { httpResponse, processedResponse }, then callers must inspect response.status or response.success rather than using try/catch to handle errors. Apply this pattern to similar network utilities (e.g., src/cdk/v2/destinations/**/utils.ts) to ensure consistent error handling and avoid exception flow in favor of structured responses.
Applied to files:
src/routes/userTransform.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Build User Transformer Docker Image - PR / Build Transformer Docker Image ARM64
- GitHub Check: Build User Transformer Docker Image - PR / Build Transformer Docker Image AMD64
- GitHub Check: Build Transformer Docker Image - PR / Build Transformer Docker Image AMD64
- GitHub Check: Build Transformer Docker Image - PR / Build Transformer Docker Image ARM64
- GitHub Check: Code Coverage
- GitHub Check: check-health
- GitHub Check: UT Tests
- GitHub Check: Check for formatting & lint errors
- GitHub Check: test_and_publish
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
|
Allure Test reports for this run are available at:
|
|
| const libraryVersionIDs = dependencies.libraries.map((lib) => lib.versionId); | ||
| const { credentials } = dependencies; | ||
|
|
||
| const response = await UserTransformService.testTransformRoutine( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need a new route if we are eventually calling same service method? why can't we use existing test endpoint ?



What are the changes introduced in this PR?
Introduce a new transformation/testRun endpoint. It currently serves as an alternative to transformation/test and will be expanded over time to become the primary endpoint for transformation testing.
Developer checklist
My code follows the style guidelines of this project
No breaking changes are being introduced.
All related docs linked with the PR?
All changes manually tested?
Any documentation changes needed with this change?
Is the PR limited to 10 file changes?
Is the PR limited to one linear task?
Are relevant unit and component test-cases added in new readability format?
Reviewer checklist
Is the type of change in the PR title appropriate as per the changes?
Verified that there are no credentials or confidential data exposed with the changes.