Test for view.prototype.render #6306
Conversation
test/app.render.js
Outdated
| @@ -365,6 +366,99 @@ describe('app', function(){ | |||
| }) | |||
| }) | |||
|
|
|||
| describe('View.prototype.render', function () { | |||
There was a problem hiding this comment.
Typically we put tests for sub systems into their own files. I have not fully read these new tests, but before I do can you move them into a new test file? The pattern for the others is to use the same file name, so in this case view.js.
test/view.js
Outdated
|
|
||
|
|
||
| describe('View.prototype.render', function () { | ||
| it('should force callback to be async and pass correct arguments', function (done) { |
There was a problem hiding this comment.
I dont think this test is correct. I would need to play around locally to be sure (which I didn't do), but your mockEngine is not asnyc and I am not sure this test even makes sense if it was. Can you describe what you are trying to prove with this isAsync part of the test?
test/view.js
Outdated
| defaultEngine: '.tmpl' | ||
| }); | ||
|
|
||
| var isAsync = false; |
There was a problem hiding this comment.
As with the one above, I dont think this isAsync stuff is doing anything meaningful. Can you describe what you are going for here to help me understand?
test/view.js
Outdated
|
|
||
| it('should pass correct arguments to the engine', function (done) { | ||
| var mockEngine = function (filePath, options, callback) { | ||
| assert.strictEqual(filePath, view.path, 'File path should match'); |
There was a problem hiding this comment.
This assertion is a self fulfilling test. If you want to test this properly you would need to hard code the expected path here, not use view.path since no matter what we changed view.path to this would pass. right?
Test for view.prototype.render
view.js (133-159 line)
Before:
After: