Skip to content

[VarDumper] Add filter in VarDumperTestTrait #22588

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
Jul 3, 2017

Conversation

maidmaid
Copy link
Contributor

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

Many casters uses EXCLUDE_* flags in their cast method. It would be usefull to use this flags directly with VarDumperTestTrait to tests many cases.

Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@maidmaid
Copy link
Contributor Author

maidmaid commented May 2, 2017

Note: in sf 4.0, methods signatures should be more elegant with $filter as 3rd param:

// sf 3.x
public function assertDumpEquals($dump, $data, $message = '', $filter = 0)
public function assertDumpMatchesFormat($dump, $data, $message = '', $filter = 0)

// sf 4.0
public function assertDumpEquals($dump, $data, $filter = 0, $message = '')
public function assertDumpMatchesFormat($dump, $data, $filter = 0, $message = '')

@nicolas-grekas
Copy link
Member

That's not possible without an upgrade path: we don't do hard BC breaks without notices first in 3.4.

@maidmaid
Copy link
Contributor Author

maidmaid commented May 2, 2017

Like this?

@trigger_error(sprintf('The $filter parameter of the %s() is deprecated since version 3.4 and will be moved at 3rd position in 4.0.', __METHOD__), E_USER_DEPRECATED);

@maidmaid maidmaid force-pushed the vardumpertesttrait-filter branch from 2768296 to 7fea536 Compare May 19, 2017 13:08
@maidmaid maidmaid changed the base branch from master to 3.4 May 19, 2017 13:08
@nicolas-grekas
Copy link
Member

@maidmaid you could detect when the type is int vs string and trigger a deprecation like in your example when relevant.

@maidmaid
Copy link
Contributor Author

Ok @nicolas-grekas, I will do this in an other PR.

@nicolas-grekas
Copy link
Member

I'd prefer in the same PR, that's the same "unit of work" to me.

@maidmaid maidmaid force-pushed the vardumpertesttrait-filter branch 4 times, most recently from dd48f24 to 9525b9a Compare June 5, 2017 21:16
@maidmaid
Copy link
Contributor Author

maidmaid commented Jun 5, 2017

Deprecation added.

@maidmaid maidmaid force-pushed the vardumpertesttrait-filter branch from 9525b9a to 1da8e71 Compare June 5, 2017 21:18
@nicolas-grekas
Copy link
Member

Thank you @maidmaid.

@nicolas-grekas nicolas-grekas merged commit 1da8e71 into symfony:3.4 Jul 3, 2017
nicolas-grekas added a commit that referenced this pull request Jul 3, 2017
This PR was merged into the 3.4 branch.

Discussion
----------

[VarDumper] Add filter in VarDumperTestTrait

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

Many casters uses ``EXCLUDE_*`` flags in their cast method. It would be usefull to use this flags directly with ``VarDumperTestTrait`` to tests many cases.

Commits
-------

1da8e71 Add filter in VarDumperTestTrait
@maidmaid maidmaid deleted the vardumpertesttrait-filter branch July 3, 2017 10:40
nicolas-grekas added a commit that referenced this pull request Jul 11, 2017
…idmaid)

This PR was merged into the 3.4 branch.

Discussion
----------

[VarDumper] Test ExceptionCaster excluding verbosity

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

Cover better ``ExceptionCaster`` in tests thanks to filter which was added in #22588.

Commits
-------

85766c9 Add exculde verbosity test
nicolas-grekas added a commit that referenced this pull request Jul 11, 2017
…aidmaid)

This PR was merged into the 3.4 branch.

Discussion
----------

[VarDumper] Test ReflectionCaster excluding verbosity

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

Cover better ``ReflectionCaster`` in tests thanks to ``EXCLUDE_*`` filter which was added in #22588.

Commits
-------

e535089 Add exculde verbosity test
symfony-splitter pushed a commit to symfony/var-dumper that referenced this pull request Jul 11, 2017
…aidmaid)

This PR was merged into the 3.4 branch.

Discussion
----------

[VarDumper] Test ReflectionCaster excluding verbosity

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

Cover better ``ReflectionCaster`` in tests thanks to ``EXCLUDE_*`` filter which was added in symfony/symfony#22588.

Commits
-------

e535089 Add exculde verbosity test
This was referenced Oct 18, 2017
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.

4 participants