Skip to content
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

feat: detailed steps output #487

Merged
merged 2 commits into from
Aug 1, 2022
Merged

feat: detailed steps output #487

merged 2 commits into from
Aug 1, 2022

Conversation

lowlighter
Copy link
Member

@lowlighter lowlighter commented Jan 12, 2022

Closes #485
This is a followup of #453

This PR introduces a detailed output:

  • each step is logged whether it succeeded or not
    • failures, trace and info are displayed on associated step with correct indentation
    • range are expanded and display current key for easier loop tracking
    • Must failed assertions display the number of skipped steps

Also fix some issues:

  • range step dump were overwritten because they had the same name
  • failures in range exited the range loop rather than fallthrough (default behaviour of steps)
  • -v and `` had actually the same verbosity level

Preview (-v output):
image

Preview (default output):
image

@nqb
Copy link
Contributor

nqb commented Jan 13, 2022

Hello @lowlighter,

Thanks for this improvement.

IMHO, we should be careful with display of variables directly in the output because they can contain sensitive informations.
I opened #476 recently and I wonder if it's not better to restrict variables into venom.log in place of STDOUT.

@fsamin fsamin requested review from fsamin and yesnault January 14, 2022 16:20
cmd/venom/run/cmd.go Outdated Show resolved Hide resolved
@ovh-cds

This comment has been minimized.

@lowlighter
Copy link
Member Author

IMHO, we should be careful with display of variables directly in the output because they can contain sensitive informations. I opened #476 recently and I wonder if it's not better to restrict variables into venom.log in place of STDOUT.

I changed it to only display the only the range key instead. The value output is good in theory, but it can be very long and break the whole output in addition of the issue you mentioned (though I think that looping over secrets is a probably idea in the start)

@lowlighter lowlighter marked this pull request as ready for review January 18, 2022 01:41
@ovh-cds

This comment has been minimized.

lowlighter added a commit that referenced this pull request Jan 24, 2022
Signed-off-by: GitHub <noreply@github.com>
@lowlighter lowlighter force-pushed the feat-steps-output branch 2 times, most recently from 146e886 to 78e2d26 Compare January 24, 2022 14:22
@ovh-cds

This comment has been minimized.

tests/verbose_output.yml Show resolved Hide resolved
cmd/venom/run/cmd.go Outdated Show resolved Hide resolved
@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 9, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

68.1% 68.1% Coverage
0.0% 0.0% Duplication

Signed-off-by: GitHub <noreply@github.com>
@ovh-cds
Copy link
Collaborator

ovh-cds commented Jul 28, 2022

CDS Report build-venom-a#892.0 ✘

  • Build
    • Build ✔
  • Tests
    • Acceptance Tests ✘

Signed-off-by: GitHub <noreply@github.com>
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

66.0% 66.0% Coverage
0.0% 0.0% Duplication

@lowlighter lowlighter changed the title feat: ranges detailed output feat: detailed steps output Jul 28, 2022
@lowlighter lowlighter requested review from fsamin and yesnault July 28, 2022 16:18
@yesnault yesnault merged commit d2680fe into master Aug 1, 2022
@yesnault yesnault deleted the feat-steps-output branch August 12, 2022 10:49
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 request: more detailed output result
5 participants