Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(264)

Issue 7196056: Added and refactored tests for ViewportModule

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 2 months ago by benji
Modified:
11 years, 2 months ago
Reviewers:
mp+145019, gary.poster
Visibility:
Public.

Description

Added and refactored tests for ViewportModule This branch adds some tests and reworks existing tests to have many fewer dependencies. This branch also introduces a couple of test helpers that make generating test doubles easier. https://code.launchpad.net/~benji/juju-gui/more-testing/+merge/145019 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 5

Patch Set 2 : Added and refactored tests for ViewportModule #

Unified diffs Side-by-side diffs Delta from patch set Stats (+175 lines, -87 lines) Patch
A [revision details] View 1 1 chunk +2 lines, -0 lines 0 comments Download
M app/views/topology/viewport.js View 1 3 chunks +49 lines, -24 lines 0 comments Download
M test/test_viewport_module.js View 1 1 chunk +108 lines, -63 lines 0 comments Download
M test/utils.js View 1 chunk +16 lines, -0 lines 0 comments Download

Messages

Total messages: 7
benji
Please take a look.
11 years, 2 months ago (2013-01-25 20:14:35 UTC) #1
matthew.scott
Thanks for the changes, land as is. One minor on a comment below (bcsaller, ping...). ...
11 years, 2 months ago (2013-01-25 20:51:55 UTC) #2
gary.poster
Cool, thank you! Gary https://codereview.appspot.com/7196056/diff/1/app/views/topology/viewport.js File app/views/topology/viewport.js (right): https://codereview.appspot.com/7196056/diff/1/app/views/topology/viewport.js#newcode43 app/views/topology/viewport.js:43: * @return {undefined} Nothing, this ...
11 years, 2 months ago (2013-01-26 03:43:51 UTC) #3
gary.poster
Sorry. I mean, please land with changes.
11 years, 2 months ago (2013-01-26 03:44:30 UTC) #4
benji
I fixed the review comments and will land. https://codereview.appspot.com/7196056/diff/1/app/views/topology/viewport.js File app/views/topology/viewport.js (right): https://codereview.appspot.com/7196056/diff/1/app/views/topology/viewport.js#newcode43 app/views/topology/viewport.js:43: * ...
11 years, 2 months ago (2013-01-28 14:14:09 UTC) #5
benji
*** Submitted: Added and refactored tests for ViewportModule This branch adds some tests and reworks ...
11 years, 2 months ago (2013-01-28 14:18:22 UTC) #6
gary.poster
11 years, 2 months ago (2013-01-28 14:18:59 UTC) #7

On Jan 28, 2013, at 9:14 AM, benji.york@gmail.com wrote:

> 
> 
>> Please don't land the XXX.
> 
> I suspect there is an argument about XXX comments I would like to have.
> ;P
> 

Heh. My request was too strong, anyway. I would like for us all to agree on an
XXX policy. Up for adding a retrospective card?

Thanks

Gary
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b