Skip to content

feat: add global output formatter support (text/json/yaml)#371

Open
cotishq wants to merge 2 commits into
microcks:masterfrom
cotishq:feat/add-output-flag-test-command
Open

feat: add global output formatter support (text/json/yaml)#371
cotishq wants to merge 2 commits into
microcks:masterfrom
cotishq:feat/add-output-flag-test-command

Conversation

@cotishq
Copy link
Copy Markdown
Contributor

@cotishq cotishq commented May 12, 2026

Description

  • Added a global --output flag to microcks test with supported values: text (default), json, and yaml.
  • Introduced a reusable output layer in pkg/output with shared formatter routing and dedicated formatters (TextFormatter, JSONFormatter, YAMLFormatter) so other commands can reuse the same pattern later.
  • Added TestResultJSON mapping structs that mirror connectors.TestResult fields (including nested test case/step fields) and used them for structured JSON/YAML serialization.
  • Updated test command execution flow to route by --output format while preserving existing default text behavior and existing exit code semantics.
  • Ensured structured outputs (json/yaml) are emitted to stdout for automation use-cases, with progress/info logs routed to stderr.
  • Added/updated tests in pkg/output for JSON/YAML formatting and formatter factory selection.
  • Updated command docs in documentation/cmd/test.md to include the new --output flag and supported values.

Related issue(s)

Resolves #301

@cotishq
Copy link
Copy Markdown
Contributor Author

cotishq commented May 12, 2026

@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

Copy link
Copy Markdown
Contributor

@Caesarsage Caesarsage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice shape overall, left a few comment you could check out

Also worth noting per the #301 discussion, this PR and #339 were meant to unify under one flag with github-actions as a value. That coordination still hasn't landed

Comment thread cmd/test.go Outdated
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")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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().

Comment thread pkg/output/formatter.go
Comment on lines +9 to +12
type Formatter interface {
FormatTestResult(result *connectors.TestResult) string
FormatTestCaseResult(testCase *connectors.TestCaseResult) string
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread cmd/test.go Outdated
Comment on lines +197 to +201
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))
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread .gitignore Outdated

# test data files
**/testdata/** No newline at end of file
**/testdata/**.gocache/
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably should be this

Suggested change
**/testdata/**.gocache/
**/testdata/**
.gocache/

@cotishq
Copy link
Copy Markdown
Contributor Author

cotishq commented May 14, 2026

Nice shape overall, left a few comment you could check out

Also worth noting per the #301 discussion, this PR and #339 were meant to unify under one flag with github-actions as a value. That coordination still hasn't landed

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.
Also thanks for the comments , i'll address all of them

@Caesarsage
Copy link
Copy Markdown
Contributor

Nice shape overall, left a few comment you could check out
Also worth noting per the #301 discussion, this PR and #339 were meant to unify under one flag with github-actions as a value. That coordination still hasn't landed

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. Also thanks for the comments , i'll address all of them

I won't advise this been merge as it is now. Check my other comments, the PR is not doing what's it stated

@cotishq
Copy link
Copy Markdown
Contributor Author

cotishq commented May 14, 2026

Nice shape overall, left a few comment you could check out
Also worth noting per the #301 discussion, this PR and #339 were meant to unify under one flag with github-actions as a value. That coordination still hasn't landed

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. Also thanks for the comments , i'll address all of them

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

@Harishrs2006
Copy link
Copy Markdown

few thoughts

  1. Moving the --output flag to rootCmd.PersistentFlags() would make
    it truly global and match the maintainer's direction from feat: Add --output flag for JSONformatted test results #301
  2. The formatter interface could accept a FormatResult(result) method
    so all branching lives inside the formatter, not the polling loop
  3. Once this lands, happy to add github-actions format as a follow-up

cotishq added 2 commits May 15, 2026 01:10
Signed-off-by: cotishq <tanishqp101204@gmail.com>
Signed-off-by: cotishq <tanishqp101204@gmail.com>
@cotishq cotishq force-pushed the feat/add-output-flag-test-command branch from b8f0c44 to 0acd8a6 Compare May 14, 2026 19:40
@cotishq
Copy link
Copy Markdown
Contributor Author

cotishq commented May 14, 2026

Nice shape overall, left a few comment you could check out
Also worth noting per the #301 discussion, this PR and #339 were meant to unify under one flag with github-actions as a value. That coordination still hasn't landed

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. Also thanks for the comments , i'll address all of them

I won't advise this been merge as it is now. Check my other comments, the PR is not doing what's it stated

i have updated the pr as you stated, i think its good now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: Add --output flag for JSONformatted test results

3 participants