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

Suppress output to stdout and stderr in copy_report_structure_spec.rb #18278

Merged

Conversation

yrudman
Copy link
Contributor

@yrudman yrudman commented Dec 10, 2018

Suppress output to stdout and stderr in copy_report_structure_spec.rb

Follow-up to #18066
Thank you @djberg96 for pointing out

BEFORE:
before

AFTER:
after

@miq-bot add-label test, gaprindashvili/yes, hammer/yes

\cc @gtanzillo

@djberg96
Copy link
Contributor

I think we need to ensure that it's reset afterwards, otherwise it affects other specs (I think). So, perhaps something like this:

before do
  old_stdout, old_stderr = $stdout, $stderr
  $stdout = $stderr = StringIO.new
end

after do
  $stdout, $stderr = old_stdout, old_stderr
end

@kbrock
Copy link
Member

kbrock commented Dec 10, 2018

@yrudman also, we do have a mechanism for testing/capturing this

expect { some code }.to output('this was printed out').to_stdout

there are a few examples in our code base

@yrudman
Copy link
Contributor Author

yrudman commented Dec 10, 2018

@djberg96 yes, need to restore original values

@kbrock it is good to know!

@yrudman yrudman force-pushed the supress-output-to-stdout-in-copyreport-rspec branch from 5059583 to d4bb1d7 Compare December 10, 2018 19:01
@yrudman yrudman force-pushed the supress-output-to-stdout-in-copyreport-rspec branch from d4bb1d7 to ab4e559 Compare December 10, 2018 19:07
@miq-bot
Copy link
Member

miq-bot commented Dec 10, 2018

Checked commit yrudman@ab4e559 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 🍪

@djberg96
Copy link
Contributor

👍

@kbrock
Copy link
Member

kbrock commented Dec 11, 2018

@yrudman If the output from this code is needed on stdout, then it seems the tests should check what is being printed out rather than just eating it.

Is it possible to add the stdout to the tests?
That way, if there is another error or something here, we catch it - rather than just eat that too

expect($stdout).to receive(:puts).with(/Copying failed/)

@Fryguy Fryguy merged commit 8157502 into ManageIQ:master Dec 11, 2018
@Fryguy Fryguy added this to the Sprint 101 Ending Dec 17, 2018 milestone Dec 11, 2018
@Fryguy Fryguy self-assigned this Dec 11, 2018
@yrudman yrudman deleted the supress-output-to-stdout-in-copyreport-rspec branch January 15, 2019 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants