-
Notifications
You must be signed in to change notification settings - Fork 2
Use outputPath parameter for JSON output instead of jsonOutputPath #25
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: main
Are you sure you want to change the base?
Conversation
Standardize JSON output to use the same outputPath parameter as JUnit XML output, removing the jsonOutputPath parameter for consistency.
WalkthroughDocumentation enhanced with JSON output examples for stdout and file writing. TestRunner.4dm adds JSON-to-file output capability with outputPath parameter parsing, introduces _writeJSONToFile function, and replaces status icons with question marks in test output. Changes
Sequence DiagramsequenceDiagram
participant User
participant TestRunner
participant JSONGen as JSON Generator
participant FileWriter as File Writer
participant Logger
User->>TestRunner: Run tests with params (outputPath)
TestRunner->>TestRunner: Parse user params
TestRunner->>JSONGen: Generate JSON report
JSONGen-->>TestRunner: JSON content
alt outputPath provided
TestRunner->>FileWriter: _writeJSONToFile(json, path)
FileWriter->>FileWriter: Create directories if needed
FileWriter->>FileWriter: Write JSON to file
FileWriter->>Logger: Log file location & summary
else outputPath not provided
TestRunner->>Logger: Log JSON to stdout
end
Logger-->>User: Output result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches🧪 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 |
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
testing/Project/Sources/Classes/TestRunner.4dm (1)
290-312: Restore distinct pass/fail glyphsLine 290 and Line 312 now emit
"? "for both passing and failing tests, so the human-readable report no longer differentiates success from failure at a glance. Please keep unique markers (e.g.,✓for pass and✗for fail) or another clear distinction.Apply this diff:
- LOG EVENT(Into system standard outputs; " ? "+$testResult.name+" ("+String($testResult.duration)+"ms)\r\n"; Information message) + LOG EVENT(Into system standard outputs; " ✓ "+$testResult.name+" ("+String($testResult.duration)+"ms)\r\n"; Information message) … - LOG EVENT(Into system standard outputs; " ? "+$testResult.name+" ("+String($testResult.duration)+"ms)"+$errorDetails+"\r\n"; Error message) + LOG EVENT(Into system standard outputs; " ✗ "+$testResult.name+" ("+String($testResult.duration)+"ms)"+$errorDetails+"\r\n"; Error message)
🧹 Nitpick comments (1)
testing/Project/Sources/Classes/TestRunner.4dm (1)
824-827: Replace the placeholder summary textLine 826 writes
"All tests passed! ??", which reads like a placeholder and weakens the success message. Please restore the prior celebratory text (e.g., the original emoji or a plain"All tests passed!"string).Apply this diff:
- LOG EVENT(Into system standard outputs; "All tests passed! ??\r\n"; Information message) + LOG EVENT(Into system standard outputs; "All tests passed! 🎉\r\n"; Information message)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
README.md(1 hunks)docs/guide.md(1 hunks)testing/Project/Sources/Classes/TestRunner.4dm(7 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
testing/Project/Sources/Classes/**/*.4dm
📄 CodeRabbit inference engine (CLAUDE.md)
testing/Project/Sources/Classes/**/*.4dm: Name 4D test classes with the suffix "Test" for auto discovery (e.g., ExampleTest.4dm)
Name test methods with the prefix "test_" so the framework recognizes and runs them
Use comment-based tagging in tests with "// #tags: tag1, tag2" to enable filtering
Opt out of parallel execution for a suite by adding "// #parallel: false" at the test class level
Disable automatic transactions for a test by adding "// #transaction: false" and manage transactions manually if needed
When manual control is required, use Testing context helpers: startTransaction, inTransaction, validateTransaction, cancelTransaction, withTransaction, withTransactionValidate
Control trigger behavior per test with annotations: "// #triggers: enabled" or "// #triggers: disabled"
Place 4D test classes under testing/Project/Sources/Classes/ so the framework can discover and run them
Tag integration tests explicitly with "// #tags: integration" (and enable triggers when needed) to support filtering
Files:
testing/Project/Sources/Classes/TestRunner.4dm
🧠 Learnings (2)
📚 Learning: 2025-10-20T18:19:46.650Z
Learnt from: CR
Repo: KyleKincer/testing PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-20T18:19:46.650Z
Learning: Applies to testing/Project/Sources/Classes/**/*.4dm : Name test methods with the prefix "test_" so the framework recognizes and runs them
Applied to files:
README.md
📚 Learning: 2025-09-22T13:15:15.427Z
Learnt from: CR
Repo: KyleKincer/testing PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T13:15:15.427Z
Learning: Applies to testing/Project/Sources/Classes/*Test.4dm : Use comment-based tagging with lines like "// #tags: unit, integration, slow" to categorize tests
Applied to files:
testing/Project/Sources/Classes/TestRunner.4dm
| $suiteResult.passed+=1 | ||
| If (This:C1470.outputFormat="human") | ||
| LOG EVENT:C667(Into system standard outputs:K38:9; " ✓ "+$testResult.name+" ("+String:C10($testResult.duration)+"ms)\r\n"; Information message:K38:1) | ||
| LOG EVENT:C667(Into system standard outputs:K38:9; " ? "+$testResult.name+" ("+String:C10($testResult.duration)+"ms)\r\n"; Information message:K38:1) |
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.
A few places n this file have changes from a symbol to ?. Is this intentional?
Standardize JSON output to use the same outputPath parameter as JUnit XML output, removing the jsonOutputPath parameter for consistency.
This change makes the API more consistent across output formats:
Both formats now use the same parameter name, improving developer experience and reducing confusion.
Summary by CodeRabbit
New Features
Documentation
Tests