Refactors Controllers to split Controllers and Routers #534
Conversation
- Improves tests and coverage, fix bugs
flovilmart
changed the title
flovilmart
changed the title
|
@flovilmart updated the pull request. |
flovilmart
changed the title
|
@flovilmart updated the pull request. |
|
Looks good so far, will take a deeper look in a bit. |
|
@nlutsenko, we should add a piece around architecture in the CONTRIBUTING.md |
|
And could improve branch coverage in the controllers to 100% as the design begs for them to be fully test drive |
| // Small additional tests to improve overall coverage | ||
| describe("FilesController",()=>{ | ||
|
|
||
| it("should properly expand objects", (done) => { |
There was a problem hiding this comment.
Are we testing the fact that the controller doesn't expand non-necessary objects?
Can we have a reverse test? Say - expand a file with just a name, so it has the full url?
There was a problem hiding this comment.
This is already covered by the integration tests that are run through Parse.File.save
|
Yup, that's why we have very good coverage on the SDKs - the controllers should be tested as much as possible... Agreed about architecture in CONTRIBUTING.md, but not sure how many people actually read it before sending a Pull Request, and in a little bit - that piece will actually disappear, since we won't need an architecture doc, as the code will look nicer 😜 Can merge as is, the question about the test is not that relevant. |
|
You can merge as is, as the coverage improves compared to the master. I'll make another PR for more tests if needed. |
|
We can make them as we go, merging now. Awesome work, sir! |
Refactors Controllers to split Controllers and Routers
@nlutsenko that's what we were talking about yesterday.