Skip to content
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

Current implementation of dumping arguments when mathod not match kills phpunit/php/everything #32

Open
jasir opened this issue Nov 4, 2014 · 6 comments

Comments

@jasir
Copy link
Contributor

jasir commented Nov 4, 2014

After couple of hours checking what is wrong (cpu high, everything freezed) I have found that implementation of throwing exception when method is wrong. Here is my (WIP) proposal - move the responsibility to Tracy.

Also there are some improvements with text of exception (see commit log).

jasir@04980a1

@janmarek
Copy link
Owner

janmarek commented Nov 5, 2014

I don't think that mockista should be dependent on Tracy just because of better exception message. Maybe we should create very simple dumper which would dump ints, strings, bools etc and print the type of other values.

@jasir
Copy link
Contributor Author

jasir commented Nov 5, 2014

Yes. That is why I did not open a pull request, just an discussion. I have thought maybe about "if tracy is present", use it, otherwise use something simple. But something different what there is in place now, which really kills everything.

@jasir
Copy link
Contributor Author

jasir commented Nov 5, 2014

Just for some impression, this is how output looks like now. Personally, I like it very much:

Unexpected call in mock eventDispatcher::dispatch(), none of 1 method(s) call candidates matched by arguments. Called with (2) arguments:
Parameter #0:"cms.cache.cleaned" (17)
Parameter #1:DNB\Events\Event #9f61
   subject private => stdClass #bef1
   |  data => stdClass #4cfb
   |  |  key => 666
   data private => NULL
   result private => NULL
   isResultSet private => FALSE
   propagationStopped private => FALSE
   dispatcher private => NULL
   name private => NULL
C:\Work\projects\cms\vendor\janmarek\mockista\Mockista\Mock.php:97
C:\Work\projects\cms\vendor\janmarek\mockista\Mockista\Mock.php:127
C:\Work\projects\cms\app\CMS\CacheManager.php:41
C:\Work\projects\cms\app\CMS\CacheManager.php:41
C:\Work\projects\cms\tests\tests\app\CMS\CacheManagerTest.php:50

@jasir
Copy link
Contributor Author

jasir commented Mar 17, 2015

What about this quick hack? Tracy\Dumper is used only if present (class_exists). It can be extended to other frameworks...

jasir@77d7f77

@MartinMystikJonas
Copy link

We just found out this problem results in false pass tests. In some cases var_dump of complex object structure kill php with exit code 0. There fore Nette tester thinks test passed.

I think we shoud:
a) use this solution
b) if dependency on Tracy have to be omitted then implement custom dumping

If opton b) would be preferred I'm willing to create PR. But in current state Mockista is unusable in about 50% of our tests.

@janmarek
Copy link
Owner

I think you can prepare PR which uses Tracy\Dumper::toText for compare the objects.

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

No branches or pull requests

3 participants