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

[VarDumper] Allow VarDumperTestTrait expectation to be non-scalar#25332

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

Conversation

@romainneutron
Copy link
Contributor

QA
Branch?master
Bug fix?no
New feature?yes
BC breaks?no
Deprecations?no
Tests pass?yes
LicenseMIT

At the moment, when using theVarDumperTestTrait in unit test, expecting data object is as follow:

class Toto{private$foo;publicfunction__construct($foo)    {$this->foo =$foo;    }}class MyTestextends \PHPUnit_Framework_TestCase{useSymfony\Component\VarDumper\Test\VarDumperTestTrait;publicfunctiondummyTest()    {$expected =<<<EOEXPECTEDProfiler\Tests\Model\CallGraph\Toto {  -foo: "baz"}EOEXPECTED;$this->assertDumpEquals($expected,newToto('baz'));    }}

The same test could be easily written like this with this change:

publicfunctiondummyTest()    {$this->assertDumpEquals(newToto('baz'),newToto('baz'));    }

@romainneutronromainneutronforce-pushed thevar-dumper-non-scalar-expectation branch from5b04647 to6b5ab90CompareDecember 5, 2017 12:52
@nicolas-grekasnicolas-grekas added this to the4.1 milestoneDec 5, 2017
@javiereguiluz
Copy link
Member

javiereguiluz commentedDec 6, 2017
edited
Loading

I'm all in for simplifications, but don't you think this is now too confusing?

$this->assertDumpEquals(newToto('baz'),newToto('baz'));

The two arguments are the same, I'm testing a dump but no dump is provided, etc.

@nicolas-grekas
Copy link
Member

@javiereguiluz that's an real improvement over the current, where you have to maintain a dump for the expected side.
The fact that dumps are compared instead of the value is what makes the "dump" word legitimate.
The failure is much improved vs standard phpunit behavior, as the diff that will be generated then will be top notch.
I confirm my 👍 personally.

@Simperfit
Copy link
Contributor

So I guess we could use this feature to improve the test in symfony ?

@fabpot
Copy link
Member

I also like this for the better exception message you get instead of the standard PHPUnit one.

@fabpot
Copy link
Member

Thank you@romainneutron.

@fabpotfabpot merged commit6b5ab90 intosymfony:masterDec 11, 2017
fabpot added a commit that referenced this pull requestDec 11, 2017
… non-scalar (romainneutron)This PR was merged into the 4.1-dev branch.Discussion----------[VarDumper] Allow VarDumperTestTrait expectation to be non-scalar| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | yes| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| License       | MITAt the moment, when using the `VarDumperTestTrait` in unit test, expecting data object is as follow:```phpclass Toto{    private $foo;    public function __construct($foo)    {        $this->foo = $foo;    }}class MyTest extends \PHPUnit_Framework_TestCase{     use Symfony\Component\VarDumper\Test\VarDumperTestTrait;    public function dummyTest()    {        $expected = <<<EOEXPECTEDProfiler\Tests\Model\CallGraph\Toto {  -foo: "baz"}EOEXPECTED;        $this->assertDumpEquals($expected, new Toto('baz'));    }}```The same test could be easily written like this with this change:```php    public function dummyTest()    {        $this->assertDumpEquals(new Toto('baz'), new Toto('baz'));    }```Commits-------6b5ab90 [VarDumper] Allow VarDumperTestTrait expectation to be non-scalar
@romainneutronromainneutron deleted the var-dumper-non-scalar-expectation branchDecember 14, 2017 10:05
@fabpotfabpot mentioned this pull requestMay 7, 2018
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@fabpotfabpotfabpot approved these changes

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

+1 more reviewer

@SimperfitSimperfitSimperfit approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

4.1

Development

Successfully merging this pull request may close these issues.

6 participants

@romainneutron@javiereguiluz@nicolas-grekas@Simperfit@fabpot@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp