From c21bdaf35deec295aac08dcc2246c6e0553f56e3 Mon Sep 17 00:00:00 2001
From: Zaven Muradyan <voithos@google.com>
Date: Sun, 23 Oct 2016 20:40:58 -0700
Subject: [PATCH] Fix bug where before/afterAll were being executed in disabled
 suites.

---
 spec/core/SuiteSpec.js                   | 21 +++------------------
 spec/core/integration/EnvSpec.js         |  2 +-
 spec/core/integration/SpecRunningSpec.js | 22 ++++++++++++++++++++++
 src/core/Env.js                          |  2 +-
 src/core/Suite.js                        | 11 +----------
 5 files changed, 28 insertions(+), 30 deletions(-)

diff --git a/spec/core/SuiteSpec.js b/spec/core/SuiteSpec.js
index 92fa59d9..f14d7089 100644
--- a/spec/core/SuiteSpec.js
+++ b/spec/core/SuiteSpec.js
@@ -83,13 +83,6 @@ describe("Suite", function() {
     expect(suite.getResult().status).toBe('finished');
   });
 
-  it("retrieves a result with disabled status", function() {
-    var suite = new jasmineUnderTest.Suite({});
-    suite.disable();
-
-    expect(suite.getResult().status).toBe('disabled');
-  });
-
   it("retrieves a result with pending status", function() {
     var suite = new jasmineUnderTest.Suite({});
     suite.pend();
@@ -97,23 +90,15 @@ describe("Suite", function() {
     expect(suite.getResult().status).toBe('pending');
   });
 
-  it("priviledges a disabled status over pending status", function() {
-    var suite = new jasmineUnderTest.Suite({});
-    suite.disable();
-    suite.pend();
-
-    expect(suite.getResult().status).toBe('disabled');
-  });
-
-  it("is executable if not disabled", function() {
+  it("is executable if not pending", function() {
     var suite = new jasmineUnderTest.Suite({});
 
     expect(suite.isExecutable()).toBe(true);
   });
 
-  it("is not executable if disabled", function() {
+  it("is not executable if pending", function() {
     var suite = new jasmineUnderTest.Suite({});
-    suite.disable();
+    suite.pend();
 
     expect(suite.isExecutable()).toBe(false);
   });
diff --git a/spec/core/integration/EnvSpec.js b/spec/core/integration/EnvSpec.js
index ecc6cb9a..bbce668c 100644
--- a/spec/core/integration/EnvSpec.js
+++ b/spec/core/integration/EnvSpec.js
@@ -1425,7 +1425,7 @@ describe("Env integration", function() {
         totalSpecsDefined: 1
       });
 
-      expect(reporter.specDone).toHaveBeenCalledWith(jasmine.objectContaining({ status: 'pending' }));
+      expect(reporter.specDone).toHaveBeenCalledWith(jasmine.objectContaining({ status: 'disabled' }));
       expect(reporter.suiteDone).toHaveBeenCalledWith(jasmine.objectContaining({ description: 'xd out', status: 'pending' }));
       expect(reporter.suiteDone.calls.count()).toBe(4);
 
diff --git a/spec/core/integration/SpecRunningSpec.js b/spec/core/integration/SpecRunningSpec.js
index d3317fcb..ad126412 100644
--- a/spec/core/integration/SpecRunningSpec.js
+++ b/spec/core/integration/SpecRunningSpec.js
@@ -518,6 +518,28 @@ describe("jasmine spec running", function () {
     env.execute();
   });
 
+  it("shouldn't run before/after functions in disabled suites", function(done) {
+    var shouldNotRun = jasmine.createSpy("shouldNotRun"),
+    suite = env.xdescribe('A disabled Suite', function() {
+      // None of the before/after functions should run.
+      env.beforeAll(shouldNotRun);
+      env.beforeEach(shouldNotRun);
+      env.afterEach(shouldNotRun);
+      env.afterAll(shouldNotRun);
+
+      env.it('spec inside a disabled suite', shouldNotRun);
+    });
+
+    var assertions = function() {
+      expect(shouldNotRun).not.toHaveBeenCalled();
+      done();
+    };
+
+    env.addReporter({jasmineDone: assertions});
+
+    env.execute();
+  });
+
   it("should allow top level suites to be disabled", function(done) {
     var specInADisabledSuite = jasmine.createSpy("specInADisabledSuite"),
       otherSpec = jasmine.createSpy("otherSpec");
diff --git a/src/core/Env.js b/src/core/Env.js
index ef5582b4..76b37832 100644
--- a/src/core/Env.js
+++ b/src/core/Env.js
@@ -229,7 +229,7 @@ getJasmineRequireObj().Env = function(j$) {
           reporter.suiteStarted(suite.result);
         },
         nodeComplete: function(suite, result) {
-          if (!suite.disabled) {
+          if (!suite.markedPending) {
             clearResourcesForRunnable(suite.id);
           }
           currentlyExecutingSuites.pop();
diff --git a/src/core/Suite.js b/src/core/Suite.js
index 459b0751..820b08e2 100644
--- a/src/core/Suite.js
+++ b/src/core/Suite.js
@@ -12,7 +12,6 @@ getJasmineRequireObj().Suite = function(j$) {
     this.afterFns = [];
     this.beforeAllFns = [];
     this.afterAllFns = [];
-    this.disabled = false;
 
     this.children = [];
 
@@ -38,10 +37,6 @@ getJasmineRequireObj().Suite = function(j$) {
     return fullName.join(' ');
   };
 
-  Suite.prototype.disable = function() {
-    this.disabled = true;
-  };
-
   Suite.prototype.pend = function(message) {
     this.markedPending = true;
   };
@@ -67,10 +62,6 @@ getJasmineRequireObj().Suite = function(j$) {
   };
 
   Suite.prototype.status = function() {
-    if (this.disabled) {
-      return 'disabled';
-    }
-
     if (this.markedPending) {
       return 'pending';
     }
@@ -83,7 +74,7 @@ getJasmineRequireObj().Suite = function(j$) {
   };
 
   Suite.prototype.isExecutable = function() {
-    return !this.disabled;
+    return !this.markedPending;
   };
 
   Suite.prototype.canBeReentered = function() {