8000 [VarDumper] ImageCaster lacks tests · Issue #33107 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[VarDumper] ImageCaster lacks tests #33107

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.

Sign up for GitHub

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

Jump to bottom
Closed
chalasr opened this issue Aug 10, 2019 · 1 comment
Closed

[VarDumper] ImageCaster lacks tests #33107

chalasr opened this issue Aug 10, 2019 · 1 comment
Labels
Good first issue Ideal for your first contribution! (some Symfony experience may be required) Help wanted Issues and PRs which are looking for volunteers to complete them. VarDumper

Comments

@chalasr
Copy link
Member
chalasr commented Aug 10, 2019 •
edited
Loading

Symfony version(s) affected: 4.4

Description
#32683 introduced a new ImageCaster on top of imagine/image.
Problem is that the feature is not tested at all, and optional features that rely on external vendors must particularly be tested (allows for early detection of incompatible versions etc..).

How to reproduce

Possible Solution
Add imagine/image to the VarDumper dev-requirements and write a unit test for the ImageCaster (see other caster test cases for inspiration).

Additional context
@chalasr chalasr added VarDumper Help wanted Issues and PRs which are looking for volunteers to complete them. Good first issue Ideal for your first contribution! (some Symfony experience may be required) labels Aug 10, 2019
@nicolas-grekas
Copy link
Member
nicolas-grekas commented Aug 10, 2019 •
edited
Loading

I don't think we want to test the ImagineCaster as it's pure glue, and not worth the additional (even dev) dependency when testing (if we start doing so, are we going to do it for all other casters? I don't think it would be useful.)

But it'd be great to test the new logic in HtmlDumper in combination with ImgStub.

@ismail1432 ismail1432 mentioned this issue Aug 16, 2019
Merged
fabpot added a commit that referenced this issue Aug 18, 2019
@fabpot
minor #33212 [VarDumper] Add test dump image (ismail1432)
63272d6 
This PR was squashed before being merged into the 4.4 branch (closes #33212).

Discussion
----------

[VarDumper] Add test dump image

| Q             | A
| ------------- | ---
| Branch?       | 4.4 <!-- see below -->
| Bug fix?      | no
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   |     <!-- please add some, will be required by reviewers -->
| Fixed tickets | #33107.   <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        |  <!-- required for new features -->

<!--
Replace this notice by a short README for your feature/bugfix. This will help people
understand your PR and can be used as a start for the documentation.

Additionally (see https://symfony.com/roadmap):
 - Bug fixes must be submitted against the lowest maintained branch where they apply
   (lowest branches are regularly merged to upper ones so they get the fixes too).
 - Features and deprecations must be submitted against branch 4.4.
 - Legacy code removals go to the master branch.
-->

Commits
-------

8393a9b [VarDumper] Add test dump image
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Good first issue Ideal for your first contribution! (some Symfony experience may be required) Help wanted Issues and PRs which are looking for volunteers to complete them. VarDumper
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@nicolas-grekas @chalasr
0