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

Some tests fail if stdin is redirected #2032

Closed
samueloph opened this issue Sep 3, 2019 · 7 comments · Fixed by #2033
Closed

Some tests fail if stdin is redirected #2032

samueloph opened this issue Sep 3, 2019 · 7 comments · Fixed by #2033

Comments

@samueloph
Copy link


Steps to reproduce

Run the tests with > /dev/null

Expected behavior

All the tests passes

Actual behavior

https://salsa.debian.org/ruby-team/capistrano/-/jobs/304495


This is a problem that we've found on Debian a little but more than 1 year ago, I didn't report it earlier because it doesn't look serious.

Some of the tests fail under our testing environment, probably because of input redirection, I started a thread on the ruby team to talk about it and this was the reply I've got from one of the team members:

I tried locally: with a simple `gbp buildpackage` the tests pass
consistently. Under sbuild, however, they fail consistently.

Looking at the failure messages, it looks like the failing tests are
somehow poking at stdin. one difference between a simple `gbp
buildpackage` and sbuild is that on the first one stdin is left alone,
while under sbuild stdin is redirected.

tried `gbp buildpackage < /dev/null` and it fails consistently

the problem seems to be in lib/capistrano/configuration/question.rb,
method `gets`. if I remove the `return unless $stdin.tty?` from there,
the tests that failed before with `< /dev/null` now pass, but then there
is a test for that specific behaviour that now fails. I think this is
something to discuss with upstream. the key to reproduce is just to run
the tests with `> /dev/null`

You can see the log of the failures here:
https://salsa.debian.org/ruby-team/capistrano/-/jobs/304495

And the patch we apply on Debian to circumvent that (disable the failing tests):
https://salsa.debian.org/ruby-team/capistrano/blob/fcd89e1042db23548771112abb8b5daabf32fc5c/debian/patches/skip_tests.patch

Thanks for making capistrano!

@mattbrictson
Copy link
Member

Thanks for the detailed report! First impression, we could make a slight change to question.rb so that $stdin could be substituted with another object for the purpose of unit testing. I'll investigate that.

@will-in-wi
Copy link
Contributor

I'd be curious if there was a way of wrapping the test execution in a bash instance so that it has a tty available…

mattbrictson added a commit that referenced this issue Sep 4, 2019
This allows the tests to run even in a situation where stdin isn't
available, for example when the test is run with `< /dev/null` as is the
case for some packaging scripts.

Fixes #2032
@mattbrictson
Copy link
Member

What do you think of this fix? #2033

mattbrictson added a commit that referenced this issue Sep 4, 2019
This allows the tests to run even in a situation where stdin isn't
available, for example when the test is run with `< /dev/null` as is the
case for some packaging scripts.

Fixes #2032
@samueloph
Copy link
Author

I added them and some tests stopped failing (13 vs 7 failures):
https://salsa.debian.org/ruby-team/capistrano/-/jobs/306437

The 7 failures are all like:
expected block to output SOMETHING to stdout, but output "\nGems\n"

So I wonder if the left ones are not related to the stdin difference but rather to something that debhelper is doing with ruby packages and its relation with gems/deps.

What do you think?

Thanks for your fast response and quick proposed solutions :)

@mattbrictson
Copy link
Member

@samueloph could you open a new issue for the "\nGems\n" errors? This seems like a slightly different problem (stdout vs stdin 🙂). What would really help is if you could document a simple way to reproduce the error. With the stdin problem I could reproduce by running the tests with < /dev/null. For the stdout case I am not sure how to trigger that.

@samueloph
Copy link
Author

@mattbrictson of course, I'm gonna gather more information about the stdout problem and then I'm gonna open a new issue for that one.

Thank you!

@samueloph
Copy link
Author

Took a look at other failures, and it looks like its related the way debhelper deals with ruby and its gems, it's doesn't look like it's gonna affect anything and I will disable these tests.

Thanks,

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 a pull request may close this issue.

3 participants