feat: add global output formatter support (text/json/yaml)#371
Conversation
ef4cdc7 to
bae1e5b
Compare
|
@Harsh4902 so i raised this pr as you stated, a global --output flag with json, yaml and text format initially, would love your on this, if changes should be make, happy to iterate |
| testCmd.Flags().StringVar(&filteredOperations, "filteredOperations", "", "List of operations to launch a test for") | ||
| testCmd.Flags().StringVar(&operationsHeaders, "operationsHeaders", "", "Override of operations headers as JSON string") | ||
| testCmd.Flags().StringVar(&oAuth2Context, "oAuth2Context", "", "Spec of an OAuth2 client context as JSON string") | ||
| testCmd.Flags().StringVar(&outputFormat, "output", "text", "Output format: text, json, or yaml") |
There was a problem hiding this comment.
The flag isn't actually global. Registered on testCmd.Flags(), not root:
microcks import-dir --output=json wouldn't work today. Worth lifting to rootCmd.PersistentFlags().
| type Formatter interface { | ||
| FormatTestResult(result *connectors.TestResult) string | ||
| FormatTestCaseResult(testCase *connectors.TestCaseResult) string | ||
| } |
There was a problem hiding this comment.
For a global flag this won't extend to import-dir or import. Either go generic (Formatter[T any]) or use per-command formatter interfaces resolved via factory.
There was a problem hiding this comment.
ya you are right that the current pkg/output.Formatter is still test-result specific, so it won’t automatically cover import/import-dir payloads yet. For this pr, I kept scope to test output behavior (issue #301) to avoid broad refactor risk.
| if isTextOutput { | ||
| fmt.Printf("MicrocksClient got status for test \"%s\" - success: %s, inProgress: %s \n", testResultID, fmt.Sprint(success), fmt.Sprint(inProgress)) | ||
| } else { | ||
| fmt.Fprintf(os.Stderr, "MicrocksClient got status for test \"%s\" - success: %s, inProgress: %s \n", testResultID, fmt.Sprint(success), fmt.Sprint(inProgress)) | ||
| } |
There was a problem hiding this comment.
Abstraction leaks back into the command. This branch repeats at three sites in the polling loop. For a global flag every command will end up with this same branching. The text/non-text routing should live in the formatter (e.g. a Progress(io.Writer, string) method) instead.
|
|
||
| # test data files | ||
| **/testdata/** No newline at end of file | ||
| **/testdata/**.gocache/ |
There was a problem hiding this comment.
probably should be this
| **/testdata/**.gocache/ | |
| **/testdata/** | |
| .gocache/ |
i stated there that , it could be good if this pr gets merged and then we apply github-actions as a value, for a clean repo state. |
I won't advise this been merge as it is now. Check my other comments, the PR is not doing what's it stated |
ya so i will address all the changes you have commented first, the PR is intended to solve the issue, but yeah your comments are worth addressing and i will come up with the valid changes |
|
few thoughts
|
bae1e5b to
b8f0c44
Compare
Signed-off-by: cotishq <tanishqp101204@gmail.com>
Signed-off-by: cotishq <tanishqp101204@gmail.com>
b8f0c44 to
0acd8a6
Compare
i have updated the pr as you stated, i think its good now |
Description
--outputflag tomicrocks testwith supported values:text(default),json, andyaml.pkg/outputwith shared formatter routing and dedicated formatters (TextFormatter, JSONFormatter, YAMLFormatter) so other commands can reuse the same pattern later.TestResultJSONmapping structs that mirrorconnectors.TestResultfields (including nested test case/step fields) and used them for structured JSON/YAML serialization.(json/yaml)are emitted tostdoutfor automation use-cases, with progress/info logs routed tostderr.documentation/cmd/test.mdto include the new--outputflag and supported values.Related issue(s)
Resolves #301