[Pkg-javascript-commits] [leaflet-markercluster] 153/219: Converted singleMarkerMode and animateOption specs

Jonas Smedegaard dr at jones.dk
Sat May 7 09:39:29 UTC 2016


This is an automated email from the git hooks/post-receive script.

js pushed a commit to branch master
in repository leaflet-markercluster.

commit 127d367c2cd6cfff6a1d5452c73cc38c70b74ae7
Author: ghybs <ghybs1 at gmail.com>
Date:   Tue Oct 20 21:59:50 2015 +0400

    Converted singleMarkerMode and animateOption specs
    
    to use back the standard beforeEach and afterEach functions, but re-using objects as much as possible. This avoids too much memory leak in PhantomJS while still having a spec/index readable.
---
 spec/suites/animateOptionSpec.js    | 121 ++++++++++++------------------------
 spec/suites/singleMarkerModeSpec.js |  72 ++++++---------------
 2 files changed, 57 insertions(+), 136 deletions(-)

diff --git a/spec/suites/animateOptionSpec.js b/spec/suites/animateOptionSpec.js
index c99ee7d..f62bf24 100644
--- a/spec/suites/animateOptionSpec.js
+++ b/spec/suites/animateOptionSpec.js
@@ -1,67 +1,33 @@
 describe('animate option', function () {
 
 	/**
-	 * Wrapper for Mocha's `it` function, to avoid using `beforeEach` and `afterEach`
-	 * which create problems with PhantomJS when total number of tests (across all suites)
-	 * increases. Might be due to use of promises for which PhantomJS would performs badly?
-	 * NOTE: works only with synchronous code.
-	 * @param testDescription string
-	 * @param testInstructions function
-	 * @param testFinally function to be executed just before afterEach2, in the `finally` block.
+	 * Avoid as much as possible creating and destroying objects for each test.
+	 * Instead, try re-using them, except for the ones under test of course.
+	 * PhantomJS does not perform garbage collection for the life of the page,
+	 * i.e. during the entire test process (Karma runs all tests in a single page).
+	 * http://stackoverflow.com/questions/27239708/how-to-get-around-memory-error-with-karma-phantomjs
+	 *
+	 * The `beforeEach` and `afterEach do not seem to cause much issue.
+	 * => they can still be used to initialize some setup between each test.
+	 * Using them keeps a readable spec/index.
+	 *
+	 * But refrain from re-creating div and map every time. Re-use those objects.
 	 */
-	function it2(testDescription, testInstructions, testFinally) {
-
-		it(testDescription, function () {
-
-			// Before each test.
-			if (typeof beforeEach2 === "function") {
-				beforeEach2();
-			}
-
-			try {
-
-				// Perform the actual test instructions.
-				testInstructions();
-
-			} catch (e) {
-
-				// Re-throw the exception so that Mocha sees the failed test.
-				throw e;
-
-			} finally {
-
-				// If specific final instructions are provided.
-				if (typeof testFinally === "function") {
-					testFinally();
-				}
-
-				// After each test.
-				if (typeof afterEach2 === "function") {
-					afterEach2();
-				}
-
-			}
-		});
-	}
-
 
 	/////////////////////////////
 	// SETUP FOR EACH TEST
 	/////////////////////////////
 
-	/**
-	 * Instructions to be executed before each test called with `it2`.
-	 */
-	function beforeEach2() {
+	beforeEach(function () {
 
-		// Nothing for this test suite.
+		previousTransitionSetting = L.DomUtil.TRANSITION;
 
-	}
+	});
 
-	/**
-	 * Instructions to be executed after each test called with `it2`.
-	 */
-	function afterEach2() {
+	afterEach(function () {
+
+		// Restore previous setting so that next tests are unaffected.
+		L.DomUtil.TRANSITION = previousTransitionSetting;
 
 		if (group instanceof L.MarkerClusterGroup) {
 			group.removeLayers(group.getLayers());
@@ -70,7 +36,8 @@ describe('animate option', function () {
 
 		// Throw away group as it can be assigned with different configurations between tests.
 		group = null;
-	}
+
+	});
 
 
 	/////////////////////////////
@@ -97,7 +64,7 @@ describe('animate option', function () {
 	// TESTS
 	/////////////////////////////
 
-	it2('hooks animated methods version by default', function () {
+	it('hooks animated methods version by default', function () {
 
 		// Need to add to map so that we have the top cluster level created.
 		group = L.markerClusterGroup().addTo(map);
@@ -120,7 +87,7 @@ describe('animate option', function () {
 
 	});
 
-	it2('hooks non-animated methods version when set to false', function () {
+	it('hooks non-animated methods version when set to false', function () {
 
 		// Need to add to map so that we have the top cluster level created.
 		group = L.markerClusterGroup({animate: false}).addTo(map);
@@ -143,41 +110,31 @@ describe('animate option', function () {
 
 	});
 
-	it2(
-		'always hooks non-animated methods version when L.DomUtil.TRANSITION is false',
-
-		function () {
-
-			previousTransitionSetting = L.DomUtil.TRANSITION;
-			// Fool Leaflet, make it think the browser does not support transitions.
-			L.DomUtil.TRANSITION = false;
+	it('always hooks non-animated methods version when L.DomUtil.TRANSITION is false', function () {
 
-			// Need to add to map so that we have the top cluster level created.
-			group = L.markerClusterGroup({animate: true}).addTo(map);
+		// Fool Leaflet, make it think the browser does not support transitions.
+		L.DomUtil.TRANSITION = false;
 
-			var noAnimation = L.MarkerClusterGroup.prototype._noAnimation;
+		// Need to add to map so that we have the top cluster level created.
+		group = L.markerClusterGroup({animate: true}).addTo(map);
 
-			// MCG non-animated methods.
-			expect(group._animationStart).to.be(noAnimation._animationStart);
-			expect(group._animationZoomIn).to.be(noAnimation._animationZoomIn);
-			expect(group._animationZoomOut).to.be(noAnimation._animationZoomOut);
-			expect(group._animationAddLayer).to.be(noAnimation._animationAddLayer);
+		var noAnimation = L.MarkerClusterGroup.prototype._noAnimation;
 
-			// MarkerCluster spiderfy non-animated methods
-			var cluster = group._topClusterLevel;
+		// MCG non-animated methods.
+		expect(group._animationStart).to.be(noAnimation._animationStart);
+		expect(group._animationZoomIn).to.be(noAnimation._animationZoomIn);
+		expect(group._animationZoomOut).to.be(noAnimation._animationZoomOut);
+		expect(group._animationAddLayer).to.be(noAnimation._animationAddLayer);
 
-			noAnimation = L.MarkerClusterNonAnimated.prototype;
+		// MarkerCluster spiderfy non-animated methods
+		var cluster = group._topClusterLevel;
 
-			expect(cluster._animationSpiderfy).to.be(noAnimation._animationSpiderfy);
-			expect(cluster._animationUnspiderfy).to.be(noAnimation._animationUnspiderfy);
+		noAnimation = L.MarkerClusterNonAnimated.prototype;
 
-		},
+		expect(cluster._animationSpiderfy).to.be(noAnimation._animationSpiderfy);
+		expect(cluster._animationUnspiderfy).to.be(noAnimation._animationUnspiderfy);
 
-		function () {
-			// Restore previous setting so that next tests are unaffected.
-			L.DomUtil.TRANSITION = previousTransitionSetting;
-		}
-	);
+	});
 
 
 	/////////////////////////////
diff --git a/spec/suites/singleMarkerModeSpec.js b/spec/suites/singleMarkerModeSpec.js
index 0c27b7d..94f4fc1 100644
--- a/spec/suites/singleMarkerModeSpec.js
+++ b/spec/suites/singleMarkerModeSpec.js
@@ -1,68 +1,31 @@
 describe('singleMarkerMode option', function () {
 
 	/**
-	 * Wrapper for Mocha's `it` function, to avoid using `beforeEach` and `afterEach`
-	 * which create problems with PhantomJS when total number of tests (across all suites)
-	 * increases. Might be due to use of promises for which PhantomJS would perform badly?
-	 * NOTE: works only with synchronous code.
-	 * @param testDescription string
-	 * @param testInstructions function
-	 * @param testFinally function to be executed just before afterEach2, in the `finally` block.
+	 * Avoid as much as possible creating and destroying objects for each test.
+	 * Instead, try re-using them, except for the ones under test of course.
+	 * PhantomJS does not perform garbage collection for the life of the page,
+	 * i.e. during the entire test process (Karma runs all tests in a single page).
+	 * http://stackoverflow.com/questions/27239708/how-to-get-around-memory-error-with-karma-phantomjs
+	 *
+	 * The `beforeEach` and `afterEach do not seem to cause much issue.
+	 * => they can still be used to initialize some setup between each test.
+	 * Using them keeps a readable spec/index.
+	 *
+	 * But refrain from re-creating div and map every time. Re-use those objects.
 	 */
-	function it2(testDescription, testInstructions, testFinally) {
-
-		it(testDescription, function () {
-
-			// Before each test.
-			if (typeof beforeEach2 === "function") {
-				beforeEach2();
-			}
-
-			try {
-
-				// Perform the actual test instructions.
-				testInstructions();
-
-			} catch (e) {
-
-				// Re-throw the exception so that Mocha sees the failed test.
-				throw e;
-
-			} finally {
-
-				// If specific final instructions are provided.
-				if (typeof testFinally === "function") {
-					testFinally();
-				}
-
-				// After each test.
-				if (typeof afterEach2 === "function") {
-					afterEach2();
-				}
-
-			}
-		});
-	}
-
 
 	/////////////////////////////
 	// SETUP FOR EACH TEST
 	/////////////////////////////
 
-	/**
-	 * Instructions to be executed before each test called with `it2`.
-	 */
-	function beforeEach2() {
+	beforeEach(function () {
 
 		// Reset the marker icon.
 		marker.setIcon(defaultIcon);
 
-	}
+	});
 
-	/**
-	 * Instructions to be executed after each test called with `it2`.
-	 */
-	function afterEach2() {
+	afterEach(function () {
 
 		if (group instanceof L.MarkerClusterGroup) {
 			group.removeLayers(group.getLayers());
@@ -71,7 +34,8 @@ describe('singleMarkerMode option', function () {
 
 		// Throw away group as it can be assigned with different configurations between tests.
 		group = null;
-	}
+
+	});
 
 
 	/////////////////////////////
@@ -102,7 +66,7 @@ describe('singleMarkerMode option', function () {
 	// TESTS
 	/////////////////////////////
 
-	it2('overrides marker icons when set to true', function () {
+	it('overrides marker icons when set to true', function () {
 
 		group = L.markerClusterGroup({
 			singleMarkerMode: true,
@@ -119,7 +83,7 @@ describe('singleMarkerMode option', function () {
 
 	});
 
-	it2('does not modify marker icons by default (or set to false)', function () {
+	it('does not modify marker icons by default (or set to false)', function () {
 
 		group = L.markerClusterGroup({
 			iconCreateFunction: function (layer) {

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-javascript/leaflet-markercluster.git



More information about the Pkg-javascript-commits mailing list