Skip to content

[Console] Fix QuestionHelperTest #14638

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

Merged
merged 1 commit into from
May 16, 2015
Merged

[Console] Fix QuestionHelperTest #14638

merged 1 commit into from
May 16, 2015

Conversation

ogizanagi
Copy link
Contributor

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -

This PR fixes the following issues in QuestionHelperTest:

  • When using the InputInterface mock with interactive option, asking a ChoiceQuestion without specifying max attempts nor a default value led to an infinite loop when invalid values are submitted. In case there is something wrong in tests, it wasn't obvious to understand what's happening, as the process was constantly waiting for inputs.
  • testAmbiguousChoiceFromChoicelist test was wrong as the autocompleter filled the value and the test was never triggered (+max attempt issue).

@fabpot
Copy link
Member

fabpot commented May 15, 2015

Looks like the problem is also present in older branches, right? In which case, this should be merged in the right branch.

@ogizanagi
Copy link
Contributor Author

@fabpot : Actually, both testSelectChoiceFromChoiceList and testAmbiguousChoiceFromChoicelist methods were not present in 2.6. Those were the only two which could be problematic when testing them.

Regarding the change L206, you're right, this could have been done in the 2.6 branch. It doesn't even have a link with this issue. It was just an unreachable assert (or at least useless if reached, because what matters is the $this->fail() part)

Should I revert this line and submit another PR for this change only, or is there anything else I missed which should be targeted in the 2.6 branch ?

@fabpot
Copy link
Member

fabpot commented May 16, 2015

@ogizanagi Yes, can you submit a PR on 2.6?

@fabpot fabpot removed the Ready label May 16, 2015
- When using the `InputInterface` mock with interactive option, asking a
  ChoiceQuestion without specifying max attempts nor a default value led
to an infinite loop when invalid values are submitted. In case there is
something wrong in tests, it wasn't obvious to understand what's
happening, as the process was constantly waiting for inputs.
- `testAmbiguousChoiceFromChoicelist` test was wrong as the
  autocompleter filled the value and the test was never triggered (+max
attempt issue).
@ogizanagi
Copy link
Contributor Author

@fabpot : Sure. I just submitted #14661 and reverted incriminated line here.

@fabpot
Copy link
Member

fabpot commented May 16, 2015

Thank you @ogizanagi.

@fabpot fabpot merged commit 6a0308d into symfony:2.7 May 16, 2015
fabpot added a commit that referenced this pull request May 16, 2015
This PR was merged into the 2.7 branch.

Discussion
----------

[Console] Fix QuestionHelperTest

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

This PR fixes the following issues in `QuestionHelperTest`:
- When using the `InputInterface` mock with interactive option, asking a ChoiceQuestion without specifying max attempts nor a default value led to an infinite loop when invalid values are submitted. In case there is something wrong in tests, it wasn't obvious to understand what's happening, as the process was constantly waiting for inputs.
- `testAmbiguousChoiceFromChoicelist` test was wrong as the autocompleter filled the value and the test was never triggered (+max attempt issue).

Commits
-------

6a0308d [Console] Fix QuestionHelperTest
@ogizanagi ogizanagi deleted the fix_tests_console_choices_max_attempts branch May 19, 2015 23:08
fabpot added a commit that referenced this pull request Jul 15, 2015
This PR was merged into the 2.7 branch.

Discussion
----------

[Console] Set QuestionHelper max attempts in tests

Otherwise the process will block if a test fails.

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

QuestionHelper tests will block the phpunit process if any of tests fails. This is what's currently happening on 2.8 branch. As soon as this is merged to 2.7 and 2.8, I can work on fixing 2.8 tests.

re #14638

Commits
-------

23bc264 [Console] Set QuestionHelper max attempts in tests
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.

3 participants