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

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

Merged
merged 1 commit into from
May 21, 2015

Conversation

watilde
Copy link
Contributor

@watilde watilde commented May 21, 2015

Add --with-test option to generate its test code scaffolding when run oil generate something.
See also: #242.

@emlynwest
Copy link
Contributor

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

@watilde
Copy link
Contributor Author

watilde commented May 21, 2015

@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
Copy link
Contributor

Ok then. Thanks.

emlynwest pushed a commit that referenced this pull request May 21, 2015
generate: Add --with-test option to create test scaffolding
@emlynwest emlynwest merged commit 06d1e99 into fuel:1.8/develop May 21, 2015
@kenjis
Copy link
Contributor

kenjis commented May 21, 2015

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

@watilde watilde deleted the feature/g-with-test branch May 21, 2015 15:01
@watilde
Copy link
Contributor Author

watilde commented May 21, 2015

@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
Copy link
Contributor

kenjis commented May 21, 2015

@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
Copy link
Contributor Author

watilde commented May 22, 2015

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

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.

None yet

3 participants