Move additional methods from Env prototype to env closure

- Users can no longer spelunk the spec tree from topSuite
- Users no longer have access to currentSuite/currentSpec
- Other miscellaneous (arguably less useful) methods have also been tucked
away into the closure, like suiteFactory

spec/core/EnvSpec.js

@@ -5,20 +5,6 @@ describe("Env", function() {
     env = new j$.Env();
   });
 
-  describe("reporting", function() {
-    var fakeReporter;
-
-    beforeEach(function() {
-      fakeReporter = jasmine.createSpyObj("fakeReporter", ["jasmineStarted"]);
-    });
-
-    it("should allow reporters to be registered", function() {
-      env.addReporter(fakeReporter);
-      env.reporter.jasmineStarted();
-      expect(fakeReporter.jasmineStarted).toHaveBeenCalled();
-    });
-  });
-
   it('removes all spies when env is executed', function(done) {
     originalFoo = function() {},
     testObj = {

src/core/Env.js

@@ -5,6 +5,8 @@ getJasmineRequireObj().Env = function(j$) {
     var self = this;
     var global = options.global || j$.getGlobal();
 
+    var totalSpecsDefined = 0;
+
     var catchExceptions = true;
 
     var realSetTimeout = j$.getGlobal().setTimeout;
@@ -15,9 +17,10 @@ getJasmineRequireObj().Env = function(j$) {
 
     var spies = [];
 
-    this.currentSpec = null;
+    var currentSpec = null;
+    var currentSuite = null;
 
-    this.reporter = new j$.ReportDispatcher([
+    var reporter = new j$.ReportDispatcher([
       "jasmineStarted",
       "jasmineDone",
       "suiteStarted",
@@ -30,7 +33,7 @@ getJasmineRequireObj().Env = function(j$) {
       return true;
     };
 
-    this.equalityTesters_ = [];
+    var equalityTesters = [];
 
     var customEqualityTesters = [];
     this.addCustomEqualityTester = function(tester) {
@@ -63,32 +66,34 @@ getJasmineRequireObj().Env = function(j$) {
     };
 
     var specStarted = function(spec) {
-      self.currentSpec = spec;
-      self.reporter.specStarted(spec.result);
+      currentSpec = spec;
+      reporter.specStarted(spec.result);
     };
 
-    var beforeFns = function(currentSuite) {
+    var beforeFns = function(suite) {
       return function() {
         var befores = [];
-        for (var suite = currentSuite; suite; suite = suite.parentSuite) {
+        while(suite) {
           befores = befores.concat(suite.beforeFns);
+          suite = suite.parentSuite;
         }
         return befores.reverse();
       };
     };
 
-    var afterFns = function(currentSuite) {
+    var afterFns = function(suite) {
       return function() {
         var afters = [];
-        for (var suite = currentSuite; suite; suite = suite.parentSuite) {
+        while(suite) {
           afters = afters.concat(suite.afterFns);
+          suite = suite.parentSuite;
         }
         return afters;
       };
     };
 
-    var getSpecName = function(spec, currentSuite) {
-      return currentSuite.getFullName() + ' ' + spec.description;
+    var getSpecName = function(spec, suite) {
+      return suite.getFullName() + ' ' + spec.description;
     };
 
     // TODO: we may just be able to pass in the fn instead of wrapping here
@@ -135,8 +140,132 @@ getJasmineRequireObj().Env = function(j$) {
       new j$.QueueRunner(options).execute();
     };
 
-    var totalSpecsDefined = 0;
-    this.specFactory = function(description, fn, suite) {
+    var topSuite = new j$.Suite({
+      env: this,
+      id: getNextSuiteId(),
+      description: 'Jasmine__TopLevel__Suite',
+      queueRunner: queueRunnerFactory,
+      completeCallback: function() {}, // TODO - hook this up
+      resultCallback: function() {} // TODO - hook this up
+    });
+    runnableLookupTable[topSuite.id] = topSuite;
+    currentSuite = topSuite;
+
+    this.execute = function(runnablesToRun) {
+      runnablesToRun = runnablesToRun || [topSuite.id];
+
+      var allFns = [];
+      for(var i = 0; i < runnablesToRun.length; i++) {
+        var runnable = runnableLookupTable[runnablesToRun[i]];
+        allFns.push((function(runnable) { return function(done) { runnable.execute(done); }; })(runnable));
+      }
+
+      reporter.jasmineStarted({
+        totalSpecsDefined: totalSpecsDefined
+      });
+
+      queueRunnerFactory({fns: allFns, onComplete: reporter.jasmineDone});
+    };
+
+    this.addReporter = function(reporterToAdd) {
+      reporter.addReporter(reporterToAdd);
+    };
+
+    this.addMatchers = function(matchersToAdd) {
+      j$.Expectation.addMatchers(matchersToAdd);
+    };
+
+    this.version = function() {
+      return j$.version;
+    };
+
+    this.versionString = function() {
+      console.log("DEPRECATED == use j$.version");
+      return j$.version;
+    };
+
+    // TODO: Still needed?
+    this.currentRunner = function() {
+      return topSuite;
+    };
+
+    this.spyOn = function(obj, methodName) {
+      if (j$.util.isUndefined(obj)) {
+        throw new Error("spyOn could not find an object to spy upon for " + methodName + "()");
+      }
+
+      if (j$.util.isUndefined(obj[methodName])) {
+        throw new Error(methodName + '() method does not exist');
+      }
+
+      if (obj[methodName] && j$.isSpy(obj[methodName])) {
+        //TODO?: should this return the current spy? Downside: may cause user confusion about spy state
+        throw new Error(methodName + ' has already been spied upon');
+      }
+
+      var spy = j$.createSpy(methodName, obj[methodName]);
+
+      spies.push({
+        spy: spy,
+        baseObj: obj,
+        methodName: methodName,
+        originalValue: obj[methodName]
+      });
+
+      obj[methodName] = spy;
+
+      return spy;
+    };
+
+    var suiteFactory = function(description) {
+      var suite = new j$.Suite({
+        env: self,
+        id: getNextSuiteId(),
+        description: description,
+        parentSuite: currentSuite,
+        queueRunner: queueRunnerFactory,
+        onStart: suiteStarted,
+        resultCallback: function(attrs) {
+          reporter.suiteDone(attrs);
+        }
+      });
+
+      runnableLookupTable[suite.id] = suite;
+      return suite;
+    };
+
+    this.describe = function(description, specDefinitions) {
+      var suite = suiteFactory(description);
+
+      var parentSuite = currentSuite;
+      parentSuite.addSuite(suite);
+      currentSuite = suite;
+
+      var declarationError = null;
+      try {
+        specDefinitions.call(suite);
+      } catch (e) {
+        declarationError = e;
+      }
+
+      if (declarationError) {
+        this.it("encountered a declaration exception", function() {
+          throw declarationError;
+        });
+      }
+
+      currentSuite = parentSuite;
+
+      return suite;
+    };
+
+    this.xdescribe = function(description, specDefinitions) {
+      var suite = this.describe(description, specDefinitions);
+      suite.disable();
+      return suite;
+    };
+
+    var specFactory = function(description, fn, suite) {
       totalSpecsDefined++;
 
       var spec = new j$.Spec({
@@ -177,180 +306,43 @@ getJasmineRequireObj().Env = function(j$) {
         removeAllSpies();
         j$.Expectation.resetMatchers();
         customEqualityTesters = [];
-        self.currentSpec = null;
-        self.reporter.specDone(result);
+        currentSpec = null;
+        reporter.specDone(result);
       }
     };
 
     var suiteStarted = function(suite) {
-      self.reporter.suiteStarted(suite.result);
+      reporter.suiteStarted(suite.result);
     };
 
-    var suiteConstructor = j$.Suite;
-
-    this.topSuite = new j$.Suite({
-      env: this,
-      id: getNextSuiteId(),
-      description: 'Jasmine__TopLevel__Suite',
-      queueRunner: queueRunnerFactory,
-      completeCallback: function() {}, // TODO - hook this up
-      resultCallback: function() {} // TODO - hook this up
-    });
-    runnableLookupTable[this.topSuite.id] = this.topSuite;
-    this.currentSuite = this.topSuite;
-
-    this.suiteFactory = function(description) {
-      var suite = new suiteConstructor({
-        env: self,
-        id: getNextSuiteId(),
-        description: description,
-        parentSuite: self.currentSuite,
-        queueRunner: queueRunnerFactory,
-        onStart: suiteStarted,
-        resultCallback: function(attrs) {
-          self.reporter.suiteDone(attrs);
-        }
-      });
-
-      runnableLookupTable[suite.id] = suite;
-      return suite;
+    this.it = function(description, fn) {
+      var spec = specFactory(description, fn, currentSuite);
+      currentSuite.addSpec(spec);
+      return spec;
     };
 
-    this.execute = function(runnablesToRun) {
-      runnablesToRun = runnablesToRun || [this.topSuite.id];
-
-      var allFns = [];
-      for(var i = 0; i < runnablesToRun.length; i++) {
-        var runnable = runnableLookupTable[runnablesToRun[i]];
-        allFns.push((function(runnable) { return function(done) { runnable.execute(done); }; })(runnable));
-      }
-
-      this.reporter.jasmineStarted({
-        totalSpecsDefined: totalSpecsDefined
-      });
-
-      queueRunnerFactory({fns: allFns, onComplete: this.reporter.jasmineDone});
+    this.xit = function(description, fn) {
+      var spec = this.it(description, fn);
+      spec.pend();
+      return spec;
     };
 
-    this.spyOn = function(obj, methodName) {
-      if (j$.util.isUndefined(obj)) {
-        throw new Error("spyOn could not find an object to spy upon for " + methodName + "()");
-      }
+    this.expect = function(actual) {
+      return currentSpec.expect(actual);
+    };
 
-      if (j$.util.isUndefined(obj[methodName])) {
-        throw new Error(methodName + '() method does not exist');
-      }
+    this.beforeEach = function(beforeEachFunction) {
+      currentSuite.beforeEach(beforeEachFunction);
+    };
 
-      if (obj[methodName] && j$.isSpy(obj[methodName])) {
-        //TODO?: should this return the current spy? Downside: may cause user confusion about spy state
-        throw new Error(methodName + ' has already been spied upon');
-      }
+    this.afterEach = function(afterEachFunction) {
+      currentSuite.afterEach(afterEachFunction);
+    };
 
-      var spy = j$.createSpy(methodName, obj[methodName]);
-
-      spies.push({
-        spy: spy,
-        baseObj: obj,
-        methodName: methodName,
-        originalValue: obj[methodName]
-      });
-
-      obj[methodName] = spy;
-
-      return spy;
+    this.pending = function() {
+      throw j$.Spec.pendingSpecExceptionMessage;
     };
   }
 
-  Env.prototype.addMatchers = function(matchersToAdd) {
-    j$.Expectation.addMatchers(matchersToAdd);
-  };
-
-  Env.prototype.version = function() {
-    return j$.version;
-  };
-
-  Env.prototype.expect = function(actual) {
-    return this.currentSpec.expect(actual);
-  };
-
-
-  // TODO: move this to closure
-  Env.prototype.versionString = function() {
-    console.log("DEPRECATED == use j$.version");
-    return j$.version;
-  };
-
-  // TODO: move this to closure
-  Env.prototype.addReporter = function(reporter) {
-    this.reporter.addReporter(reporter);
-  };
-
-  // TODO: move this to closure
-  Env.prototype.describe = function(description, specDefinitions) {
-    var suite = this.suiteFactory(description);
-
-    var parentSuite = this.currentSuite;
-    parentSuite.addSuite(suite);
-    this.currentSuite = suite;
-
-    var declarationError = null;
-    try {
-      specDefinitions.call(suite);
-    } catch (e) {
-      declarationError = e;
-    }
-
-    if (declarationError) {
-      this.it("encountered a declaration exception", function() {
-        throw declarationError;
-      });
-    }
-
-    this.currentSuite = parentSuite;
-
-    return suite;
-  };
-
-  // TODO: move this to closure
-  Env.prototype.xdescribe = function(description, specDefinitions) {
-    var suite = this.describe(description, specDefinitions);
-    suite.disable();
-    return suite;
-  };
-
-  // TODO: move this to closure
-  Env.prototype.it = function(description, fn) {
-    var spec = this.specFactory(description, fn, this.currentSuite);
-    this.currentSuite.addSpec(spec);
-    return spec;
-  };
-
-  // TODO: move this to closure
-  Env.prototype.xit = function(description, fn) {
-    var spec = this.it(description, fn);
-    spec.pend();
-    return spec;
-  };
-
-  // TODO: move this to closure
-  Env.prototype.beforeEach = function(beforeEachFunction) {
-    this.currentSuite.beforeEach(beforeEachFunction);
-  };
-
-  // TODO: move this to closure
-  Env.prototype.afterEach = function(afterEachFunction) {
-    this.currentSuite.afterEach(afterEachFunction);
-  };
-
-  // TODO: move this to closure
-  Env.prototype.pending = function() {
-    throw j$.Spec.pendingSpecExceptionMessage;
-  };
-
-  // TODO: Still needed?
-  Env.prototype.currentRunner = function() {
-    return this.topSuite;
-  };
-
   return Env;
 };