Refs #2378: noted $group bugs; code format fairy

README.md

@@ -113,3 +113,4 @@ Here is a list of global items that I know about that may need to be done in the
   * Consider ditching `PipelineD` entirely here; might be more confusing than helpful and can still achieve the same results with ease
   * UNWRAP ALL CLASSES SINCE THIS IS NODE AFTERALL (A BUILD STEP COULD REWRAP THEM FOR BROWSER-LAND IF NEEDED)
   * MAKE THIS WORK IN A BROWSER -- THAT'D BE NICE
+  * $group and $group.$addToSet both use JSON.stringify for key checks but really need a deepEqual (via Document.compare) or maybe use jsonPlus (faster?) ... fix me now!

lib/pipeline/Value.js

@@ -101,7 +101,7 @@ var Value = module.exports = Value = (function(){
 		// Numbers
 		if (lt === "number" && rt === "number"){
 			//NOTE: deviation from Mongo code: they handle NaN a bit differently
-			if (isNaN(l)) return isNaN(r)?0:-1;
+			if (isNaN(l)) return isNaN(r) ? 0 : -1;
 			if (isNaN(r)) return 1;
 			return l < r ? -1 : l > r ? 1 : 0;
 		}

lib/pipeline/accumulators/AddToSetAccumulator.js

@@ -8,11 +8,11 @@ var AddToSetAccumulator = module.exports = (function(){
 	 * @module munge
 	 * @constructor
 	**/
-	var klass = module.exports = function AddToSetAccumulator(/* pCtx */){
+	var klass = module.exports = function AddToSetAccumulator(/* ctx */){
 		if(arguments.length !== 0) throw new Error("zero args expected");
 		this.set = {};
 		//this.itr = undefined; /* Shoudln't need an iterator for the set */
-		//this.pCtx = undefined; /* Not using the context object currently as it is related to sharding */
+		//this.ctx = undefined; /* Not using the context object currently as it is related to sharding */
 		base.call(this);
 	}, Accumulator = require("./Accumulator"), base = Accumulator, proto = klass.prototype = Object.create(base.prototype, {constructor:{value:klass}});
 

lib/pipeline/documentSources/CursorDocumentSource.js

@@ -25,8 +25,8 @@ var CursorDocumentSource = module.exports = (function(){
 //		source.  Therefore, bson dependencies must appear before pCursor
 //		in order cause its destructor to be called *after* pCursor's.
 //		*/
-//		this.pQuery = null;
-//		this.pSort = null;
+//		this.query = null;
+//		this.sort = null;
 
 		this._projection = null;
 

lib/pipeline/documentSources/FilterBaseDocumentSource.js

@@ -15,15 +15,14 @@ var FilterBaseDocumentSource = module.exports = (function(){
 		base.call(this, ctx);
 		this.unstarted = true;
 		this.hasNext = false;
-		this.pCurrent = null;
+		this.current = null;
 	}, base = require('./DocumentSource'), proto = klass.prototype = Object.create(base.prototype, {constructor:{value:klass}});
 
-	//! @todo Need to implement coalesce()
-	//! @todo Need to implement optimize()
+	//TODO: Need to implement coalesce()
+	//TODO: Need to implement optimize()
 
 	/**
 	 * Find the next acceptable source document, if there are any left.
-	 *
 	 * @method findNext
 	**/
 	proto.findNext = function findNext() {
@@ -34,27 +33,26 @@ var FilterBaseDocumentSource = module.exports = (function(){
 		}
 
 		while(this.hasNext) {
-			var pDocument = this.source.getCurrent();
+			var document = this.source.getCurrent();
 			this.hasNext = this.source.advance();
 
-			if (this.accept(pDocument)) {
-				this.pCurrent = pDocument;
+			if (this.accept(document)) {
+				this.current = document;
 				return;
 			}
 		}
 
-		this.pCurrent = null;
+		this.current = null;
 	};
 
 	/**
 	 * Is the source at EOF?
-	 *
 	 * @method	eof
 	**/
 	proto.eof = function eof() {
 		if (this.unstarted)
 			this.findNext();
-		return (this.pCurrent === null);
+		return (this.current === null);
 	};
 
 	/**
@@ -78,7 +76,7 @@ var FilterBaseDocumentSource = module.exports = (function(){
 		* first will skip over the first item.
 		**/
 		this.findNext();
-		return (this.pCurrent !== null);
+		return (this.current !== null);
 	};
 
 	/**
@@ -90,18 +88,18 @@ var FilterBaseDocumentSource = module.exports = (function(){
 	proto.getCurrent = function getCurrent() {
 		if (this.unstarted)
 			this.findNext();
-		if (this.pCurrent === null) throw new Error("This should never happen");
-		return this.pCurrent;
+		if (this.current === null) throw new Error("This should never happen");
+		return this.current;
 	};
 
 	/**
 	* Test the given document against the predicate and report if it
 	* should be accepted or not.
 	*
-	* @param {object} pDocument the document to test
+	* @param {object} document the document to test
 	* @returns {bool} true if the document matches the filter, false otherwise
 	**/
-	proto.accept = function accept(pDocument) {
+	proto.accept = function accept(document) {
 		throw new Error("not implemented");
 	};
 
@@ -114,9 +112,9 @@ var FilterBaseDocumentSource = module.exports = (function(){
 	* format.  This conversion is used to move back to the low-level
 	* find() Cursor mechanism.
 	*
-	* @param pBuilder the builder to write to
+	* @param builder the builder to write to
 	**/
-	proto.toMatcherJson = function toMatcherJson(pBuilder) {
+	proto.toMatcherJson = function toMatcherJson(builder) {
 		throw new Error("not implemented");
 	};
 

lib/pipeline/documentSources/GroupDocumentSource.js

@@ -18,7 +18,7 @@ var GroupDocumentSource = module.exports = (function(){
 	 * @constructor
 	 * @param [ctx] {ExpressionContext}
 	 **/
-	var klass = module.exports = GroupDocumentSource = function GroupDocumentSource(expCtx){
+	var klass = module.exports = GroupDocumentSource = function GroupDocumentSource(expCtx) {
 		if(arguments.length > 1) throw new Error("up to one arg expected");
 		base.call(this, expCtx);
 
@@ -47,7 +47,7 @@ var GroupDocumentSource = module.exports = (function(){
 	};
 
 	klass.groupName = "$group";
-	proto.getSourceName = function getSourceName(){
+	proto.getSourceName = function getSourceName() {
 		return klass.groupName;
 	};
 
@@ -80,39 +80,36 @@ var GroupDocumentSource = module.exports = (function(){
 	};
 
 	klass.createFromJson = function createFromJson(groupObj, ctx) {
-		if(!(groupObj instanceof Object && groupObj.constructor.name === "Object")) throw new Error("a group's fields must be specified in an object");
+		if (!(groupObj instanceof Object && groupObj.constructor === Object)) throw new Error("a group's fields must be specified in an object");
 
 		var idSet = false,
 			group = new GroupDocumentSource(ctx);
 
-		for(var groupFieldName in groupObj){
-			if(groupObj.hasOwnProperty(groupFieldName)){
+		for (var groupFieldName in groupObj) {
+			if (groupObj.hasOwnProperty(groupFieldName)) {
 				var groupField = groupObj[groupFieldName];
 
-				if(groupFieldName === "_id"){
+				if (groupFieldName === "_id") {
 
-					if(idSet) {
-						throw new Error("15948 a group's _id may only be specified once");
-					}
+					if(idSet) throw new Error("15948 a group's _id may only be specified once");
 
-					if(groupField instanceof Object && groupField.constructor.name === "Object"){
+					if (groupField instanceof Object && groupField.constructor === Object) {
 						var objCtx = new Expression.ObjectCtx({isDocumentOk:true});
 						group.idExpression = Expression.parseObject(groupField, objCtx);
 						idSet = true;
 
-					}else if( typeof groupField === "string"){
-						if(groupField[0] !== "$") {
+					} else if (typeof groupField === "string") {
+						if (groupField[0] !== "$") {
 							group.idExpression = new ConstantExpression(groupField);
-						}
-						else {
+						} else {
 							var pathString = Expression.removeFieldPrefix(groupField);
 							group.idExpression = new FieldPathExpression(pathString);
 						}
-
 						idSet = true;
-					}else{
+
+					} else {
 						var typeStr = group._getTypeStr(groupField);
-						switch(typeStr){
+						switch (typeStr) {
 							case "number":
 							case "string":
 							case "boolean":
@@ -128,30 +125,26 @@ var GroupDocumentSource = module.exports = (function(){
 					}
 
 
-				}else{
-					if(groupFieldName.indexOf(".") !== -1)
-						throw new Error("16414 the group aggregate field name '" + groupFieldName + "' cannot contain '.'");
-					if(groupFieldName[0] === "$")
-						throw new Error("15950 the group aggregate field name '" + groupFieldName + "' cannot be an operator name");
-					if(group._getTypeStr(groupFieldName) === "Object")
-						throw new Error("15951 the group aggregate field '" + groupFieldName + "' must be defined as an expression inside an object");
+				} else {
+					if (groupFieldName.indexOf(".") !== -1) throw new Error("16414 the group aggregate field name '" + groupFieldName + "' cannot contain '.'");
+					if (groupFieldName[0] === "$") throw new Error("15950 the group aggregate field name '" + groupFieldName + "' cannot be an operator name");
+					if (group._getTypeStr(groupFieldName) === "Object") throw new Error("15951 the group aggregate field '" + groupFieldName + "' must be defined as an expression inside an object");
 
 					var subFieldCount = 0;
-					for(var subFieldName in groupField){
-						if(groupField.hasOwnProperty(subFieldName)){
+					for (var subFieldName in groupField) {
+						if (groupField.hasOwnProperty(subFieldName)) {
 							var subField = groupField[subFieldName],
 								op = klass.groupOps[subFieldName];
-							if(!op)
-								throw new Error("15952 unknown group operator '" + subFieldName + "'");
+							if (!op) throw new Error("15952 unknown group operator '" + subFieldName + "'");
 
 							var groupExpression,
 								subFieldTypeStr = group._getTypeStr(subField);
-							if(subFieldTypeStr === "Object"){
+							if (subFieldTypeStr === "Object") {
 								var subFieldObjCtx = new Expression.ObjectCtx({isDocumentOk:true});
 								groupExpression = Expression.parseObject(subField, subFieldObjCtx);
-							}else if(subFieldTypeStr === "Array"){
+							} else if (subFieldTypeStr === "Array") {
 								throw new Error("15953 aggregating group operators are unary (" + subFieldName + ")");
-							}else{
+							} else {
 								groupExpression = Expression.parseOperand(subField);
 							}
 							group.addAccumulator(groupFieldName,op, groupExpression);
@@ -159,35 +152,30 @@ var GroupDocumentSource = module.exports = (function(){
 							++subFieldCount;
 						}
 					}
-					if(subFieldCount != 1)
-						throw new Error("15954 the computed aggregate '" + groupFieldName + "' must specify exactly one operator");
+					if (subFieldCount != 1) throw new Error("15954 the computed aggregate '" + groupFieldName + "' must specify exactly one operator");
 				}
 			}
 		}
 
-		if(!idSet) {
-			throw new Error("15955 a group specification must include an _id");
-		}
+		if (!idSet) throw new Error("15955 a group specification must include an _id");
 
 		return group;
 	};
 
-	proto._getTypeStr = function _getTypeStr(obj){
-		var typeofStr=typeof obj,
-			typeStr=((typeofStr == "object" && obj !== null ) ? obj.constructor.name : typeofStr);
-
+	proto._getTypeStr = function _getTypeStr(obj) {
+		var typeofStr = typeof obj,
+			typeStr = (typeofStr == "object" && obj !== null) ? obj.constructor.name : typeofStr;
 		return typeStr;
 	};
 
-	proto.advance = function advance(){
+	proto.advance = function advance() {
 		base.prototype.advance.call(this); // Check for interupts ????
-		if(!this.populated)
-			this.populate();
+		if(!this.populated) this.populate();
 
 		//verify(this.currentGroupsKeysIndex < this.groupsKeys.length);
 
 		++this.currentGroupsKeysIndex;
-		if(this.currentGroupsKeysIndex === this.groupsKeys.length){
+		if (this.currentGroupsKeysIndex === this.groupsKeys.length) {
 			this.currentDocument = null;
 			return false;
 		}
@@ -196,18 +184,13 @@ var GroupDocumentSource = module.exports = (function(){
 		return true;
 	};
 
-	proto.eof = function eof(){
-		if(!this.populated)
-			this.populate();
-
+	proto.eof = function eof() {
+		if (!this.populated) this.populate();
 		return this.currentGroupsKeysIndex === this.groupsKeys.length;
-
 	};
 
-	proto.getCurrent = function getCurrent(){
-		if(!this.populated)
-			this.populate();
-
+	proto.getCurrent = function getCurrent() {
+		if (!this.populated) this.populate();
 		return this.currentDocument;
 	};
 
@@ -216,38 +199,36 @@ var GroupDocumentSource = module.exports = (function(){
 		// add _id
 		this.idExpression.addDependencies(deps);
 		// add the rest
-		this.fieldNames.forEach(function(field, i) {
+		this.fieldNames.forEach(function (field, i) {
 			self.expressions[i].addDependencies(deps);
 		});
 
 		return DocumentSource.GetDepsReturn.EXHAUSTIVE;
 	};
 
-	proto.addAccumulator = function addAccumulator(fieldName, accumulatorFactory, expression){
+	proto.addAccumulator = function addAccumulator(fieldName, accumulatorFactory, expression) {
 		this.fieldNames.push(fieldName);
 		this.accumulatorFactories.push(accumulatorFactory);
 		this.expressions.push(expression);
 	};
 
-	proto.populate = function populate(){
-		for(var hasNext = !this.source.eof(); hasNext; hasNext = this.source.advance()){
+	proto.populate = function populate() {
+		for (var hasNext = !this.source.eof(); hasNext; hasNext = this.source.advance()) {
 			var group,
 				currentDocument = this.source.getCurrent(),
 				_id = this.idExpression.evaluate(currentDocument);
 
-			if(undefined === _id) {
-				_id = null;
-			}
+			if (undefined === _id) _id = null;
 
-			var idHash = JSON.stringify(_id); //! @todo USE A REAL HASH.  I didn't have time to take collision into account.
+			var idHash = JSON.stringify(_id); //TODO: USE A REAL HASH.  I didn't have time to take collision into account.
 
-			if(idHash in this.groups){
+			if (idHash in this.groups) {
 				group = this.groups[idHash];
-			}else{
+			} else {
 				this.groups[idHash] = group = [];
 				this.groupsKeys[this.currentGroupsKeysIndex] = idHash;
 				++this.currentGroupsKeysIndex;
-				for(var ai =0; ai < this.accumulatorFactories.length; ++ai){
+				for (var ai = 0; ai < this.accumulatorFactories.length; ++ai) {
 					var accumulator = new this.accumulatorFactories[ai]();
 					accumulator.addOperand(this.expressions[ai]);
 					group.push(accumulator);
@@ -256,29 +237,31 @@ var GroupDocumentSource = module.exports = (function(){
 
 
 			// tickle all the accumulators for the group we found
-			for(var gi=0; gi < group.length; ++gi)
+			for (var gi = 0; gi < group.length; ++gi) {
 				group[gi].evaluate(currentDocument);
+			}
 
 		}
 
 		this.currentGroupsKeysIndex = 0; // Start the group
-		if(this.groupsKeys.length > 0)
+		if (this.groupsKeys.length > 0) {
 			this.currentDocument = this.makeDocument(this.currentGroupsKeysIndex);
+		}
 		this.populated = true;
 
 	};
 
-	proto.makeDocument = function makeDocument(groupKeyIndex){
+	proto.makeDocument = function makeDocument(groupKeyIndex) {
 		var groupKey = this.groupsKeys[groupKeyIndex],
 			group = this.groups[groupKey],
 			doc = {};
 
 		doc[Document.ID_PROPERTY_NAME] = JSON.parse(groupKey);
 
-		for(var i = 0; i < this.fieldNames.length; ++i){
+		for (var i = 0; i < this.fieldNames.length; ++i) {
 			var fieldName = this.fieldNames[i],
 				item = group[i];
-			if((item !== "null") && (typeof item !== "undefined")){
+			if (item !== "null" && item !== undefined) {
 				doc[fieldName] = item.getValue();
 			}
 		}

lib/pipeline/documentSources/LimitDocumentSource.js

@@ -76,7 +76,7 @@ var LimitDocumentSource = module.exports = (function(){
 		if (this.count >= this.limit) {
 			return false;
 		}
-		this.pCurrent = this.source.getCurrent();
+		this.current = this.source.getCurrent();
 		return this.source.advance();
 	};
 

lib/pipeline/documentSources/SkipDocumentSource.js

@@ -47,11 +47,11 @@ var SkipDocumentSource = module.exports = (function(){
 		}
 
 		if (this.source.eof()) {
-			this.pCurrent = null;
+			this.current = null;
 			return;
 		}
 
-		this.pCurrent = this.source.getCurrent();
+		this.current = this.source.getCurrent();
 	};
 
 
@@ -87,13 +87,12 @@ var SkipDocumentSource = module.exports = (function(){
 	proto.advance = function advance() {
 		base.prototype.advance.call(this); // check for interrupts
 		if (this.eof()) {
-			this.pCurrent = null;
+			this.current = null;
 			return false;
 		}
 
-		this.pCurrent = this.source.getCurrent();
+		this.current = this.source.getCurrent();
 		return this.source.advance();
-
 	};
 
 	/**

test/lib/pipeline/Pipeline.js

@@ -58,20 +58,20 @@ module.exports = {
 
 		"parseCommand": {
 
-			"should fail if given non-objects in the array": function () {
+			"should throw Error if given non-objects in the array": function () {
 				assert.throws(function(){
 					Pipeline.parseCommand({pipeline:[5]});
 				});
 			},
 
-			"should fail if given objects with more/less than one field": function () {
+			"should throw Error if given objects with more / less than one field": function () {
 				assert.throws(function(){
 					Pipeline.parseCommand({pipeline:[{}]});
 					Pipeline.parseCommand({pipeline:[{a:1,b:2}]});
 				});
 			},
 
-			"should fail if given objects that dont match any known document sources": function () {
+			"should throw Error on unknown document sources": function () {
 				assert.throws(function(){
 					Pipeline.parseCommand({pipeline:[{$foo:"$sdfdf"}]});
 				});
@@ -105,19 +105,18 @@ module.exports = {
 
 			"should set the parent source for all sources in the pipeline except the first one": function (next) {
 				var p = Pipeline.parseCommand({pipeline:[{$test:{coalesce:false}}, {$test:{coalesce:false}}, {$test:{coalesce:false}}]});
-				p.run(new DocumentSource({}), function(err, result){
+				p.run(new DocumentSource({}), function(err, results){
 					assert.equal(p.sourceVector[1].source, p.sourceVector[0]);
 					assert.equal(p.sourceVector[2].source, p.sourceVector[1]);
-					next();
+					return next();
 				});
 			},
 
 			"should iterate through sources and return resultant array": function (next) {
-				var p = Pipeline.parseCommand({pipeline:[{$test:{coalesce:false}}, {$test:{coalesce:false}}, {$test:{coalesce:false}}]}),
-					result = {};
-				p.run(new DocumentSource({}), function(err, result){
-					assert.deepEqual(result.result, [5,4,3,2,1,0]);//see the test source for why this should be so
-					next();
+				var p = Pipeline.parseCommand({pipeline:[{$test:{coalesce:false}}, {$test:{coalesce:false}}, {$test:{coalesce:false}}]});
+				p.run(new DocumentSource({}), function(err, results){
+					assert.deepEqual(results.result, [5,4,3,2,1,0]); //see the test source for why this should be so
+					return next();
 				});
 			},
 

test/lib/pipeline/documentSources/GroupDocumentSource.js

@@ -35,17 +35,16 @@ function assertExpectedResult(args) {
 		assert.deepEqual(result, args.expected);
 		checkJsonRepresentation(gds, args.spec);
 	}else{
-		if(args.throw)
+		if(args.throw) {
 			assert.throws(function(){
 				GroupDocumentSource.createFromJson(args.spec);
 			});
-		else
+		} else {
 			assert.doesNotThrow(function(){
 				var gds = GroupDocumentSource.createFromJson(args.spec);
 				checkJsonRepresentation(gds, args.spec);
 			});
-
-
+		}
 	}
 }
 
@@ -56,27 +55,27 @@ module.exports = {
 
 		"constructor()": {
 
-			// $group spec is not an object. g
+			// $group spec is not an object
 			"should throw Error when constructing without args": function testConstructor(){
 				assertExpectedResult({"throw":true});
 			},
 
-			// $group spec is not an object. g
+			// $group spec is not an object
 			"should throw Error when $group spec is not an object": function testConstructor(){
 				assertExpectedResult({spec:"Foo"});
 			},
 
-			// $group spec is an empty object. g
+			// $group spec is an empty object
 			"should throw Error when $group spec is an empty object": function testConstructor(){
 				assertExpectedResult({spec:{}});
 			},
 
-			// $group _id is an empty object. g
+			// $group _id is an empty object
 			"should not throw when _id is an empty object": function advanceTest(){
 				assertExpectedResult({spec:{_id:{}}, "throw":false});
 			},
 
-			// $group _id is specified as an invalid object expression. g
+			// $group _id is specified as an invalid object expression
 			"should throw error when  _id is an invalid object expression": function testConstructor(){
 				assertExpectedResult({
 					spec:{_id:{$add:1, $and:1}},
@@ -84,73 +83,73 @@ module.exports = {
 			},
 
 
-			// $group with two _id specs. g
+			// $group with two _id specs
 			//NOT Implemented can't do this in Javascript
 
 
 
-			// $group _id is the empty string. g
+			// $group _id is the empty string
 			"should not throw when _id is an empty string": function advanceTest(){
 				assertExpectedResult({spec:{_id:""}, "throw":false});
 			},
 
-			// $group _id is a string constant. g
+			// $group _id is a string constant
 			"should not throw when _id is a string constant": function advanceTest(){
 				assertExpectedResult({spec:{_id:"abc"}, "throw":false});
 			},
 
-			// $group with _id set to an invalid field path. g
+			// $group with _id set to an invalid field path
 			"should throw when _id is an invalid field path": function advanceTest(){
 				assertExpectedResult({spec:{_id:"$a.."}});
 			},
 		
-			// $group _id is a numeric constant. g
+			// $group _id is a numeric constant
 			"should not throw when _id is a numeric constant": function advanceTest(){
 				assertExpectedResult({spec:{_id:2}, "throw":false});
 			},
 
-			// $group _id is an array constant. g
+			// $group _id is an array constant
 			"should not throw when _id is an array constant": function advanceTest(){
 				assertExpectedResult({spec:{_id:[1,2]}, "throw":false});
 			},
 
-			// $group _id is a regular expression (not supported). g
+			// $group _id is a regular expression (not supported)
 			"should throw when _id is a regex": function advanceTest(){
 				assertExpectedResult({spec:{_id:/a/}});
 			},
 
-			// The name of an aggregate field is specified with a $ prefix. g
+			// The name of an aggregate field is specified with a $ prefix
 			"should throw when aggregate field spec is specified with $ prefix": function advanceTest(){
 				assertExpectedResult({spec:{_id:1, $foo:{$sum:1}}});
 			},
 
-			// An aggregate field spec that is not an object. g
+			// An aggregate field spec that is not an object
 			"should throw when aggregate field spec is not an object": function advanceTest(){
 				assertExpectedResult({spec:{_id:1, a:1}});
 			},
 
-			// An aggregate field spec that is not an object. g
+			// An aggregate field spec that is not an object
 			"should throw when aggregate field spec is an empty object": function advanceTest(){
 				assertExpectedResult({spec:{_id:1, a:{}}});
 			},
 
-			// An aggregate field spec with an invalid accumulator operator. g
+			// An aggregate field spec with an invalid accumulator operator
 			"should throw when aggregate field spec is an invalid accumulator": function advanceTest(){
 				assertExpectedResult({spec:{_id:1, a:{$bad:1}}});
 			},
 
-			// An aggregate field spec with an array argument. g
+			// An aggregate field spec with an array argument
 			"should throw when aggregate field spec with an array as an argument": function advanceTest(){
 				assertExpectedResult({spec:{_id:1, a:{$sum:[]}}});
 			},
 
-			// Multiple accumulator operators for a field. g
+			// Multiple accumulator operators for a field
 			"should throw when aggregate field spec with multiple accumulators": function advanceTest(){
 				assertExpectedResult({spec:{_id:1, a:{$sum:1, $push:1}}});
 			}
 
 			//Not Implementing, not way to support this in Javascript Objects
-			// Aggregation using duplicate field names is allowed currently. g
+			// Aggregation using duplicate field names is allowed currently
 
 
 
@@ -166,58 +165,58 @@ module.exports = {
 
 		"#advance": {
 
-			// $group _id is computed from an object expression. g
-			"should compute _id from an object expression": function advanceTest(){
+			// $group _id is computed from an object expression
+			"should compute _id from an object expression": function testAdvance_ObjectExpression(){
 				assertExpectedResult({
-					docs:[{a:6}],
-					spec:{_id:{z:"$a"}},
-					expected:{_id:{z:6}}
+					docs: [{a:6}],
+					spec: {_id:{z:"$a"}},
+					expected: {_id:{z:6}}
 				});
 			},
 
-			// $group _id is a field path expression. g
-			"should compute _id a field path expression": function advanceTest(){
+			// $group _id is a field path expression
+			"should compute _id from a field path expression": function testAdvance_FieldPathExpression(){
 				assertExpectedResult({
-					docs:[{a:5}],
-					spec:{_id:"$a"},
-					expected:{_id:5}
+					docs: [{a:5}],
+					spec: {_id:"$a"},
+					expected: {_id:5}
 				});
 			},
 			
-			// $group _id is a field path expression. g
-			"should compute _id a field path expression if expression evaluates to a Date": function advanceDateTest(){
+			// $group _id is a field path expression
+			"should compute _id from a Date": function testAdvance_Date(){
 				var d = new Date();
 				assertExpectedResult({
-					docs:[{a:d}],
-					spec:{_id:"$a"},
-					expected:{_id:d}
+					docs: [{a:d}],
+					spec: {_id:"$a"},
+					expected: {_id:d}
 				});
 			},
 
-			// Aggregate the value of an object expression. g
-			"should aggregate the value of an object expression": function advanceTest(){
+			// Aggregate the value of an object expression
+			"should aggregate the value of an object expression": function testAdvance_ObjectExpression(){
 				assertExpectedResult({
-					docs:[{a:6}],
-					spec:{_id:0, z:{$first:{x:"$a"}}},
-					expected:{_id:0, z:{x:6}}
+					docs: [{a:6}],
+					spec: {_id:0, z:{$first:{x:"$a"}}},
+					expected: {_id:0, z:{x:6}}
 				});
 			},
 
-//			// Aggregate the value of an operator expression. g
-			"should aggregate the value of an operator expression": function advanceTest(){
+			// Aggregate the value of an operator expression
+			"should aggregate the value of an operator expression": function testAdvance_OperatorExpression(){
 				assertExpectedResult({
-					docs:[{a:6}],
-					spec:{_id:0, z:{$first:"$a"}},
-					expected:{_id:0, z:6}
+					docs: [{a:6}],
+					spec: {_id:0, z:{$first:"$a"}},
+					expected: {_id:0, z:6}
 				});
 			},
 
-			// Aggregate the value of an operator expression. g
-			"should aggregate the value of an operator expression with a null id": function nullTest(){
+			// Aggregate the value of an operator expression
+			"should aggregate the value of an operator expression with a null id": function testAdvance_Null(){
 				assertExpectedResult({
-					docs:[{a:6}],
-					spec:{_id:null, z:{$first:"$a"}},
-					expected:{_id:null, z:6}
+					docs: [{a:6}],
+					spec: {_id:null, z:{$first:"$a"}},
+					expected: {_id:null, z:6}
 				});
 			}