From 1e13039478b66961bda33f686f53cee82a6f1e4e Mon Sep 17 00:00:00 2001
From: Elliot Nelson <Elliot.Nelson@hbo.com>
Date: Fri, 17 May 2019 11:43:22 -0400
Subject: [PATCH] spyOnProperty respects the allowRespy flag

---
 spec/core/SpyRegistrySpec.js | 50 ++++++++++++++++++++++++------------
 src/core/SpyRegistry.js      |  9 ++++---
 2 files changed, 40 insertions(+), 19 deletions(-)

diff --git a/spec/core/SpyRegistrySpec.js b/spec/core/SpyRegistrySpec.js
index c7f53fe1..dcf414a2 100644
--- a/spec/core/SpyRegistrySpec.js
+++ b/spec/core/SpyRegistrySpec.js
@@ -137,22 +137,6 @@ describe("SpyRegistry", function() {
       }).toThrowError(/does not have access type/);
     });
 
-    it("checks if it has already been spied upon", function() {
-      var spyRegistry = new jasmineUnderTest.SpyRegistry({createSpy: createSpy}),
-        subject = {};
-
-      Object.defineProperty(subject, 'spiedProp', {
-        get: function() { return 1; },
-        configurable: true
-      });
-
-      spyRegistry.spyOnProperty(subject, 'spiedProp');
-
-      expect(function() {
-        spyRegistry.spyOnProperty(subject, 'spiedProp');
-      }).toThrowError(/has already been spied upon/);
-    });
-
     it("checks if it can be spied upon", function() {
       var subject = {};
 
@@ -212,6 +196,40 @@ describe("SpyRegistry", function() {
       expect(subject.spiedProperty).toEqual(returnValue);
       expect(setter).toEqual(spy);
     });
+
+    describe("when the property is already spied upon", function() {
+      it("throws an error if respy is not allowed", function() {
+        var spyRegistry = new jasmineUnderTest.SpyRegistry({createSpy: createSpy}),
+          subject = {};
+
+        Object.defineProperty(subject, 'spiedProp', {
+          get: function() { return 1; },
+          configurable: true
+        });
+
+        spyRegistry.spyOnProperty(subject, 'spiedProp');
+
+        expect(function() {
+          spyRegistry.spyOnProperty(subject, 'spiedProp');
+        }).toThrowError(/spiedProp#get has already been spied upon/);
+      });
+
+      it("returns the original spy if respy is allowed", function() {
+        var spyRegistry = new jasmineUnderTest.SpyRegistry({createSpy: createSpy}),
+          subject = {};
+
+        spyRegistry.allowRespy(true);
+
+        Object.defineProperty(subject, 'spiedProp', {
+          get: function() { return 1; },
+          configurable: true
+        });
+
+        var originalSpy = spyRegistry.spyOnProperty(subject, 'spiedProp');
+
+        expect(spyRegistry.spyOnProperty(subject, 'spiedProp')).toBe(originalSpy);
+      });
+    });
   });
 
   describe("#spyOnAllFunctions", function() {
diff --git a/src/core/SpyRegistry.js b/src/core/SpyRegistry.js
index 595ce461..90d93490 100644
--- a/src/core/SpyRegistry.js
+++ b/src/core/SpyRegistry.js
@@ -27,7 +27,7 @@ getJasmineRequireObj().SpyRegistry = function(j$) {
       }
 
       if (obj[methodName] && j$.isSpy(obj[methodName])  ) {
-        if ( !!this.respy ){
+        if (this.respy) {
           return obj[methodName];
         }else {
           throw new Error(getErrorMsg(methodName + ' has already been spied upon'));
@@ -91,8 +91,11 @@ getJasmineRequireObj().SpyRegistry = function(j$) {
       }
 
       if (j$.isSpy(descriptor[accessType])) {
-        //TODO?: should this return the current spy? Downside: may cause user confusion about spy state
-        throw new Error(propertyName + ' has already been spied upon');
+        if (this.respy) {
+          return descriptor[accessType];
+        } else {
+          throw new Error(propertyName + '#' + accessType + ' has already been spied upon');
+        }
       }
 
       var originalDescriptor = j$.util.clone(descriptor),