From 89e2679f08010918e8e1dd04f126cfa9ecea5070 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alexandre=20K=C3=BChn?= <aku@odoo.com>
Date: Thu, 22 Feb 2018 09:48:25 +0100
Subject: [PATCH] [FIX] web: tests: turn patchWindowGetSelection stateless

test_utils.js contains helper functions, which should be stateless
(good practices). This was not the case with patchWindowGetSelection
and unpatchWindowGetSelection, so instead we keep only the first
function which returns the second one.

From the test POV, instead of this:

    utils.patchWindowGetSelection();
    // ...
    utils.unpatchWindowGetSelection();

It now looks like that:

    var unpatchWindowGetSelection = utils.patchWindowGetSelection();
    // ...
    unpatchWindowGetSelection();
---
 addons/mail/static/tests/chatter_tests.js     |  5 ++--
 addons/mail/static/tests/discuss_tests.js     |  3 +--
 addons/web/static/tests/helpers/test_utils.js | 23 +++++++------------
 3 files changed, 11 insertions(+), 20 deletions(-)

diff --git a/addons/mail/static/tests/chatter_tests.js b/addons/mail/static/tests/chatter_tests.js
index e247c01430a..16bc619e8c7 100644
--- a/addons/mail/static/tests/chatter_tests.js
+++ b/addons/mail/static/tests/chatter_tests.js
@@ -370,7 +370,7 @@ QUnit.test('chatter: post, receive and star messages', function (assert) {
     var done = assert.async();
     assert.expect(28);
     
-    testUtils.patchWindowGetSelection();
+    var unpatchWindowGetSelection = testUtils.patchWindowGetSelection();
 
     var bus = new Bus();
     var BusService = createBusService(bus);
@@ -538,8 +538,7 @@ QUnit.test('chatter: post, receive and star messages', function (assert) {
 
             //cleanup
             form.destroy();
-            testUtils.unpatchWindowGetSelection();
-            
+            unpatchWindowGetSelection();
             done();
         });
 });
diff --git a/addons/mail/static/tests/discuss_tests.js b/addons/mail/static/tests/discuss_tests.js
index 348a6357b5d..550b7d46b72 100644
--- a/addons/mail/static/tests/discuss_tests.js
+++ b/addons/mail/static/tests/discuss_tests.js
@@ -12,7 +12,6 @@ var BasicComposer = Composers.BasicComposer;
 var createBusService = mailTestUtils.createBusService;
 var createDiscuss = mailTestUtils.createDiscuss;
 var patchWindowGetSelection = testUtils.patchWindowGetSelection;
-var unpatchWindowGetSelection = testUtils.unpatchWindowGetSelection;
 
 QUnit.module('mail', {}, function () {
 
@@ -113,7 +112,7 @@ QUnit.test('@ mention in channel', function (assert) {
         // Note: focus is needed in order to trigger rpc 'channel_fetch_listeners'
         $input.focus();
         $input.text("@");
-        patchWindowGetSelection();
+        var unpatchWindowGetSelection = patchWindowGetSelection();
         $input.trigger('keyup');
 
         fetchListenersDef
diff --git a/addons/web/static/tests/helpers/test_utils.js b/addons/web/static/tests/helpers/test_utils.js
index 6f3b3ef4549..5b2fdf2e7f6 100644
--- a/addons/web/static/tests/helpers/test_utils.js
+++ b/addons/web/static/tests/helpers/test_utils.js
@@ -23,8 +23,6 @@ var Widget = require('web.Widget');
 
 var DebouncedField = basic_fields.DebouncedField;
 
-var originalWindowGetSelection; // cached replaced originalGetSelection
-
 /**
  * intercepts an event bubbling up the widget hierarchy. The event intercepted
  * must be a "custom event", i.e. an event generated by the method 'trigger_up'.
@@ -734,21 +732,22 @@ function patch (target, props) {
  * 
  * usage:
  *     QUnit.test('...',function(done){
- *          testUtils.patchWindowGetSelection();
+ *          var unpatchWindowGetSelection = testUtils.patchWindowGetSelection();
  *          
  *          // do something that needs to use window.getSelection()
  *          assert.strictEqual(....);
  *          
  *          // restore the original function
- *          testUtils.unpatchWindowGetSelection();
+ *          unpatchWindowGetSelection();
  *      
  *          // finish the test
  *          done();
  *     })
+ *
+ * @returns {function} the unpatch function
  */
-
 function patchWindowGetSelection() {
-    originalWindowGetSelection = window.getSelection;
+    var originalWindowGetSelection = window.getSelection;
     window.getSelection = function () {
         return {
             removeAllRanges: function () {},
@@ -769,6 +768,9 @@ function patchWindowGetSelection() {
             },
         };
     };
+    return function () {
+        window.getSelection = originalWindowGetSelection;
+    };
 }
 
 /**
@@ -795,14 +797,6 @@ function unpatch(target) {
     delete target.__patchID;
 }
 
-/**
- * after using patchWindowGetSelection, at the end of your test
- * you must restore the default function so there is no impact to the other tests
- */
-function unpatchWindowGetSelection() {
-    window.getSelection = originalWindowGetSelection;
-}
-
 // Loading static files cannot be properly simulated when their real content is
 // really needed. This is the case for static XML files so we load them here,
 // before starting the qunit test suite.
@@ -835,7 +829,6 @@ return $.when(
         triggerMouseEvent: triggerMouseEvent,
         triggerPositionalMouseEvent: triggerPositionalMouseEvent,
         unpatch: unpatch,
-        unpatchWindowGetSelection: unpatchWindowGetSelection,
     };
 });