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

modified tests, improved configuration, introduced aspect #1714

Merged
merged 4 commits into from
Dec 31, 2013
Merged

modified tests, improved configuration, introduced aspect #1714

merged 4 commits into from
Dec 31, 2013

Conversation

Ragazzo
Copy link
Contributor

@Ragazzo Ragazzo commented Dec 30, 2013

I improved current testing support by adding AspectMock and Specify. What benefits we get:

  1. AspectMock allows us to mock directly to the class. Great ability. Can be compared to mockery but it uses class aliases and can cause fatal error when same name class will be included.
  2. Specify syntax sugar that adds good readability. Also it is a best practice to structure in blocks your assertions, so they will be easy to read and understand.

This two repos are maintained by @DavertMik so we will get support from him i think. Also you can review and see by yourself that tests are easy to use/read/understand.

I made some _bootstrap.php modifications because of AspectMock should be initialized before Yii, and load it internally.

@Ragazzo
Copy link
Contributor Author

Ragazzo commented Dec 30, 2013

@qiangxue i do really think that this changes should be implemented because of they are very useful for developers especially who are starting to learn TDD for example.

@Ragazzo
Copy link
Contributor Author

Ragazzo commented Dec 30, 2013

Related with this issue #1709 and this one #1694 as example of how it can help developer to mock things.

@qiangxue
Copy link
Member

AspectMock should be initialized before Yii, and load it internally.

Why?

@Ragazzo
Copy link
Contributor Author

Ragazzo commented Dec 30, 2013

It is Go! AOP framework (created by @lisachenko) limitation, generally it is because of how AOP work. Some links:

@Ragazzo
Copy link
Contributor Author

Ragazzo commented Dec 30, 2013

I think we also can wait @DavertMik as a creator of AspectMock for bringing up some internal knowledges.

@DavertMik
Copy link
Contributor

I'd rather leave AspectMock aside. Sure it is really effective tool especially for unit testing in Yii and the only option in such cases as form testing here goes a link to my post on that.

But I'd rather not introduce this into Yii2 core. Just because it is still is (and looks like ever be) experimental framework which does a black magic. It will work for most of cases, but we can't provide a support in cases when it doesn't work, works in improper way, or even worst - affects the application in production.

AspectMock will be developed and supported. It's a really useful tool. But I'm not sure it may fit to everyone,


Even though I can recommend specify as it makes tests more descriptive and readable. They are simple and sweet and can bring much better structure into classical unit tests.

@Ragazzo
Copy link
Contributor Author

Ragazzo commented Dec 31, 2013

Ok, then will make changes and exclude AspectMock from current PR. Will mention it in docs guide for testing.

@Ragazzo
Copy link
Contributor Author

Ragazzo commented Dec 31, 2013

@qiangxue i think we should raise method visibility LoginForm::getUser to public (for testing purposes too) :)

@qiangxue
Copy link
Member

Feel free to do so if needed.

@lisachenko
Copy link

I agree with @DavertMik that AspectMock is not very stable yet, because of techniques that are used in Go! AOP framework. However, AspectMock is very promising and interesting project, but it requires more attention and testing to be stable and predictable in all cases. So, it will be better to switch to standard testing framework to be sure that everything will be ok.

BTW, Yii framework can be integrated will go-aop to use an aspect-oriented programming. I have a post about this: http://go.aopphp.com/blog/2013/09/28/aspect-oriented-programming-with-yii/

@Ragazzo
Copy link
Contributor Author

Ragazzo commented Dec 31, 2013

Reverted aspect-mock changes. Done, ready for review.

qiangxue added a commit that referenced this pull request Dec 31, 2013
modified tests, improved configuration, introduced aspect
@qiangxue qiangxue merged commit 53daaf8 into yiisoft:master Dec 31, 2013
@qiangxue
Copy link
Member

Cool! Thanks!

@Ragazzo Ragazzo deleted the tests_improved branch December 31, 2013 19:17
@Ragazzo
Copy link
Contributor Author

Ragazzo commented Dec 31, 2013

Great :) Still need to improve db support)

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

Successfully merging this pull request may close these issues.

4 participants