Skip to content

Conversation

ogizanagi
Copy link
Contributor

@ogizanagi ogizanagi commented Nov 19, 2019

Q A
Branch? 3.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets N/A
License MIT
Doc PR N/A

Spotted while working on #34456.
Not sure it should really qualify as a bugfix, but the ContextualValidatorInterface::validate method is expected to return the instance. If chaining in a validator, trying to use this test methods would throw an error, trying to call a method on null.

@chalasr
Copy link
Member

chalasr commented Nov 23, 2019

Thank you @ogizanagi.

chalasr pushed a commit that referenced this pull request Nov 23, 2019
…n value to mocked validate method calls (ogizanagi)

This PR was merged into the 3.4 branch.

Discussion
----------

[Validator] ConstraintValidatorTestCase: add missing return value to mocked validate method calls

| Q             | A
| ------------- | ---
| Branch?       | 3.4 <!-- see below -->
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | N/A <!-- prefix each issue number with "Fix #", if any -->
| License       | MIT
| Doc PR        | N/A

Spotted while working on #34456.
Not sure it should really qualify as a bugfix, but the `ContextualValidatorInterface::validate` method is expected to return the instance. If [chaining in a validator](https://github.com/symfony/symfony/pull/34456/files#diff-0e6e3106aa637d750d47e86a14cef8d4R43), trying to use this test methods would throw an error, trying to call a method on `null`.

Commits
-------

8d1f326 [Validator] ConstraintValidatorTestCase: add missing return value to mocked validate method calls
@chalasr chalasr merged commit 8d1f326 into symfony:3.4 Nov 23, 2019
@ogizanagi ogizanagi deleted the validator_test_case_mock_return_value branch November 23, 2019 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants