Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[Process] Dont test TTY if there is no TTY support#38950

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
nicolas-grekas merged 1 commit intosymfony:4.4fromNyholm:tty
Nov 2, 2020

Conversation

@Nyholm
Copy link
Member

QA
Branch?4.4
Bug fix?no
New feature?no
Deprecations?no
TicketsFix#38946
LicenseMIT
Doc PR

@derrabus
Copy link
Member

Well, this would make our tests pass. But my understanding is that the TTY detection that you use to skip the test is part of what we want to test here. Let's say we would break that detection logic. We would never know because the corresponding tests would flip from green to skipped. 😕

@Nyholm
Copy link
MemberAuthor

Is it testingProcess::isTtySupported()?

Isn't it testing that theProcess is usingUnixPipes and handle the exit code correctly?

@Nyholm
Copy link
MemberAuthor

PR updated. Thank you

@derrabus
Copy link
Member

@Nyholm If I changeProcess::isTtySupported() to always returnfalse, those two tests are the only ones failing.

@Nyholm
Copy link
MemberAuthor

IfProcess::isTtySupported() always returnsfalse, then these two tests will be skipped.

IfProcess::isTtySupported() always returns true, yes, these will fail.

What alternative strategy do you have to solve#38946?

@derrabus
Copy link
Member

What alternative strategy do you have to solve#38946?

None. 😕

@nicolas-grekas
Copy link
Member

Thank you@Nyholm.

@nicolas-grekasnicolas-grekas merged commited217a1 intosymfony:4.4Nov 2, 2020
@NyholmNyholm deleted the tty branchNovember 3, 2020 16:14
@fabpotfabpot mentioned this pull requestNov 10, 2020
This was referencedNov 29, 2020
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@derrabusderrabusderrabus left review comments

@fabpotfabpotfabpot requested changes

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

4.4

Development

Successfully merging this pull request may close these issues.

5 participants

@Nyholm@derrabus@nicolas-grekas@fabpot@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp