fuel / oil

Fuel PHP Framework - Fuel v1.x Oil command-line package
http://fuelphp.com/docs/packages/oil/intro.html
106 stars 67 forks source link

generate: Add --with-test option to create test scaffolding #243

Closed watilde closed 9 years ago

watilde commented 9 years ago

Add --with-test option to generate its test code scaffolding when run oil generate something. See also: https://github.com/fuel/oil/issues/242.

emlynwest commented 9 years ago

Just curious, why would you test a view? https://github.com/fuel/oil/pull/243/files#diff-f010a25c6b4710011f6a51b4bd650285R850

watilde commented 9 years ago

@stevewest It's useful to make sure that the input data is identical when you do refactoring, and also there is a .gitkeep in test/view: https://github.com/fuel/fuel/tree/1.8/develop/fuel/app/tests/view

emlynwest commented 9 years ago

Ok then. Thanks.

kenjis commented 9 years ago

Oh, test/view in fuel/fuel was for view model, so it must be renamed to test/presenter.

watilde commented 9 years ago

@stevewest Thanks!

@kenjis Thanks for you comment! Any docs of the object that test/view was only for view model? Surely, I was thinking the test for actually using something method like render or get of the View Class.

kenjis commented 9 years ago

@watilde Probably there is no docs. But I was created folders in https://github.com/fuel/fuel/tree/1.8/develop/fuel/app/tests as exactly the same names in https://github.com/fuel/fuel/tree/1.8/develop/fuel/app/classes

app/classes/controller -> tesst/controller
app/classes/model      -> tests/model
app/classes/view       -> tests/view

As you know, view was renamed to presenter.

I thought we must write tests for classes, so I created three folders. And I have not written tests for view class. But if you write tests for view class, we have no reason to stop it. Now we have presenter, so it may be okay tests/view is for view class.

tesst/controller
tests/model
tests/presenter
tests/view

I don't have strong opinion to this.

watilde commented 9 years ago

@kenjis Ok I'll update this patch, I agree with you for now. Thanks :smiley_cat: