launchpad-reviewers team mailing list archive

["j.c.sackett" <jcsackett@xxxxxxxxxxxxx>]

j.c.sackett has proposed merging lp:~jcsackett/launchpad/comment-js-test-coverage into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/comment-js-test-coverage/+merge/115165

Summary
=======
The comment js module had very little test coverage. This remedies that.

This still doesn't cover everything--progress UI is test inasmuch as it's
ensured that various things call it; I focused on the important bits of the of
functionality so that refactors can safely happen.

Implementation
==============
Added tests.

Tests
=====
bin/test -vvct comment --layer=YUI

QA
==
No qa. This just adds test coverage. If tests pass, qa is done.

LoC
===
This is under the disclosure arc.

Lint
====

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/app/javascript/tests/test_comment.js
  lib/lp/app/javascript/testing/helpers.js
  lib/lp/app/javascript/comment.js
-- 
https://code.launchpad.net/~jcsackett/launchpad/comment-js-test-coverage/+merge/115165
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/comment-js-test-coverage into lp:launchpad.

=== modified file 'lib/lp/app/javascript/comment.js'
--- lib/lp/app/javascript/comment.js	2012-07-03 17:10:03 +0000
+++ lib/lp/app/javascript/comment.js	2012-07-16 14:52:27 +0000
@@ -134,7 +134,9 @@
         var comment = Y.Node.create(message_html);
         fieldset.get('parentNode').insertBefore(comment, fieldset);
         this.reset_contents();
-        Y.lp.anim.green_flash({node: comment}).run();
+        if (this.get('animate')) {
+            Y.lp.anim.green_flash({node: comment}).run();
+        }
     },
     /**
      * Reset the widget to a blank state.
@@ -195,7 +197,8 @@
     ATTRS: {
         lp_client: {
             valueFn: function() { return new Y.lp.client.Launchpad() } 
-        } 
+        },
+        animate: { value: true }
     },
 });
 

=== modified file 'lib/lp/app/javascript/testing/helpers.js'
--- lib/lp/app/javascript/testing/helpers.js	2012-06-28 22:33:35 +0000
+++ lib/lp/app/javascript/testing/helpers.js	2012-07-16 14:52:27 +0000
@@ -32,6 +32,9 @@
         this.patch = function(bug_filter, data, config) {
             this._call('patch', config, arguments);
         };
+        this.get = function(url, config) {
+            this._call('get', config, arguments);
+        };
     }
     ns.LPClient.prototype._call = function(name, config, args) {
         this.received.push(

=== modified file 'lib/lp/app/javascript/tests/test_comment.js'
--- lib/lp/app/javascript/tests/test_comment.js	2012-07-03 18:53:00 +0000
+++ lib/lp/app/javascript/tests/test_comment.js	2012-07-16 14:52:27 +0000
@@ -30,24 +30,91 @@
             Y.Assert.isFalse(comment.validate());
         },
 
-        test_post_comment: function () {
+        _get_mocked_comment: function () {
             window.LP = {
                 cache: {
                     bug: { self_link: '/bug/1/' }
                 }
             };
             var mock_client = new Y.lp.testing.helpers.LPClient();
+            mock_client.named_post.args = [];
+            mock_client.get.args = [];
             var comment = new Y.lp.app.comment.Comment(
-                { lp_client:  mock_client });
-            var no_op = function () {};
-            mock_client.named_post.args = [];
+                {
+                    animate: false,
+                    lp_client: mock_client
+                });
+            return comment;
+        },
+
+        test_add_comment_calls: function () {
+            var comment = this._get_mocked_comment();
+            var progress_ui_called = false;
+            var post_comment_called = false;
+            var extra_call_called = false;
+            comment.activateProgressUI = function () {
+                progress_ui_called = true;
+            };
+            comment.post_comment = function () {
+                post_comment_called = true;
+            };
+            comment._add_comment_success = function () {
+                extra_call_called = true;
+            };
+            comment.validate = function () { return true };
+            var mock_event = { halt: function () {} };
+
+            comment.add_comment(mock_event);
+            Y.Assert.isTrue(progress_ui_called);
+            Y.Assert.isTrue(post_comment_called);
+        },
+
+        test_get_comment_html: function () {
+            var comment = this._get_mocked_comment();
+            var message_entry = {
+                get: function (ignore) {
+                    // `get` is only called with `self_link` in this function.
+                    // If that changes, this will need to be mocked better.
+                    return "https://example.com/comments/3";;
+                }
+            };
+            var callback_called = false;
+            var callback = function () {
+                callback_called = true;
+            };
+            comment.get_comment_HTML(message_entry, callback);
+            Y.Assert.isTrue(callback_called);
+            var client = comment.get('lp_client');
+            var url = client.received[0][1][0].split("?")[0];
+            Y.Assert.areEqual(url, 'https://example.com/comments/3');
+        },
+
+        test_post_comment: function () {
+            var comment = this._get_mocked_comment();
+            var no_op_called = false;
+            var no_op = function () {
+                no_op_called = true;
+            };
             comment.post_comment(no_op);
+            var client = comment.get('lp_client');
             Y.Assert.areEqual(
-                mock_client.received[0][1][0],
+                client.received[0][1][0],
                 '/bug/1/');
             Y.Assert.areEqual(
-                mock_client.received[0][1][1],
+                client.received[0][1][1],
                 'newMessage');
+            Y.Assert.isTrue(no_op_called);
+        },
+
+        test_insert_comment_html: function () {
+            var comment = this._get_mocked_comment();
+            var reset_contents_called = false;
+            comment.reset_contents = function () {
+                reset_contents_called = true; 
+            };
+            comment.insert_comment_HTML('<span id="fake"></span>');
+            Y.Assert.isTrue(reset_contents_called);
+            Y.Assert.isObject(Y.one('#fake'));
         }
     }));