EAGLESIX-2699 All the CondExpression test cases are passing. Improved IfNullExpression test cases. NotExpression test cases are passing.

CondExpression test cases are fragile.  IfNull test cases require cleanup.

lib/pipeline/expressions/CondExpression.js

@@ -29,6 +29,13 @@ proto.getOpName = function getOpName() {
     return klass.opName;
 };
 
+/**
+ * A utility class. Not in the c++ code.
+ * @param v				a value that should be tested for being null-ish.
+ * @returns {boolean}
+ */
+klass.isNullish = function(op) {return (op.value == null || op.value == undefined) && !op._fieldPath};
+
 /**
  *
  * @param expr	- I expect this to be the RHS of $cond:{...} or $cond:[,,,]
@@ -41,13 +48,14 @@ klass.parse = function parse(expr, vps) {
 
     // if not an object, return;
 	// todo I don't understand why we'd do this.  shouldn't expr be {}, [], or wrong?
-    if (typeof(expr) !== Object || )
+    if (typeof(expr) !== 'object')
 		return FixedArityExpressionT.parse(expr, vps);
 
-	// ...or expr could be the entirety of $cond:{...} or $cond:[,,,].
-	if(!(klass.opName in expr)) {
-		throw new Error("Invalid expression. Expected to see '"+klass.opName+"'");
-	}
+//NOTE: Deviation from Mongo: The c++ code has a verify( $cond ) structure. The implementation below will never work.
+//	// ...or expr could be the entirety of $cond:{...} or $cond:[,,,].
+//	if(!(klass.opName in expr)) {
+//		throw new Error("Invalid expression. Expected to see '"+klass.opName+"'");
+//	}
 
     var ret = new CondExpression();
 
@@ -57,7 +65,8 @@ klass.parse = function parse(expr, vps) {
 	// parseOperand (again) without altering the input.)
 //    var args = Expression.parseOperand(expr, vps);
 
-	var args = expr[getOpName()];
+//	var args = expr[klass.opName];
+	var args = expr;
 
 	if (typeof args !== 'object') throw new Error("this should not happen");
 	if (args instanceof Array) {
@@ -82,9 +91,9 @@ klass.parse = function parse(expr, vps) {
 		}
 	});
 
-    if (!ret.operands[0]) throw new Error("Missing 'if' parameter to $cond; code 17080");
-    if (!ret.operands[1]) throw new Error("Missing 'then' parameter to $cond; code 17081");
-    if (!ret.operands[2]) throw new Error("Missing 'else' parameter to $cond; code 17082");
+    if (klass.isNullish(ret.operands[0])) throw new Error("Missing 'if' parameter to $cond; code 17080");
+    if (klass.isNullish(ret.operands[1])) throw new Error("Missing 'then' parameter to $cond; code 17081");
+    if (klass.isNullish(ret.operands[2])) throw new Error("Missing 'else' parameter to $cond; code 17082");
 
     return ret;
 };
@@ -100,7 +109,7 @@ proto.evaluateInternal = function evaluateInternal(vars) {
 		var pCond1 = this.operands[0].evaluateInternal(vars);
 
 		this.idx = 0;
-		if (pCond1.coerceToBool()) {
+		if (Value.coerceToBool(pCond1)) {
 			this.idx = 1;
 		} else {
 			this.idx = 2;

test/lib/pipeline/expressions/CondExpression_test.js

@@ -1,6 +1,8 @@
 "use strict";
 var assert = require("assert"),
 	CondExpression = require("../../../../lib/pipeline/expressions/CondExpression"),
+	VariablesParseState = require("../../../../lib/pipeline/expressions/VariablesParseState"),
+	VariablesIdGenerator = require("../../../../lib/pipeline/expressions/VariablesIdGenerator"),
 	Expression = require("../../../../lib/pipeline/expressions/Expression");
 
 
@@ -74,7 +76,8 @@ module.exports = {
 						assert.throws(function(){
 							Expression.parseOperand(expr, {});
 						});
-					}
+					};
+					this.vps = new VariablesParseState(new VariablesIdGenerator());
 				},
 				"should fail because the $cond is missing": function(){
 					this.shouldFail({$zoot:[true, 1, 0 ]}, {});
@@ -107,17 +110,17 @@ module.exports = {
 				},
 				"should evaluate true even with mixed up args": function(){
 					assert.strictEqual(
-						Expression.parseOperand({$cond:{ else: 0, then: 1, if: "$a" }}, {}).evaluate({$a: 1}),
+						Expression.parseOperand({$cond:{ else: 0, then: 1, if: "$a" }}, this.vps).evaluate({a: 1}),
 						1);
 				},
 				"should evaluate false": function(){
 					assert.strictEqual(
-						Expression.parseOperand({$cond:{ if: "$a", then: 0, else: 1}}, {}).evaluate({$a: 0}),
+						Expression.parseOperand({$cond:{ if: "$a", then: 0, else: 1}}, this.vps).evaluate({a: 0}),
 						1);
 				},
 				"should evaluate false even with mixed up args": function() {
 					assert.strictEqual(
-						Expression.parseOperand({$cond: { else: 1, then: 0, if: "$a"}}, {}).evaluate({$a: 0}),
+						Expression.parseOperand({$cond: { else: 1, then: 0, if: "$a"}}, this.vps).evaluate({a: 0}),
 						1);
 				}
 			}

test/lib/pipeline/expressions/IfNullExpression_test.js

@@ -37,25 +37,16 @@ module.exports = {
 			beforeEach: function () {
 				this.vps = new VariablesParseState(new VariablesIdGenerator());
 				this.parsed = Expression.parseExpression("$ifNull", ["$a", "$b"], this.vps);
-				this.vars = new Variables(2);
-				this.vars.setValue(0, "a");
-				this.vars.setValue(1, "b");
-				this.makeParsed = function(a, b) {
-					return Expression.parseExpression("$ifNull", [a, b], this.vps);
-				}
 			},
 
 			"should return the left hand side if the left hand side is not null or undefined": function() {
-				//assert.strictEqual(this.parsed.evaluate(this.vars), 1);
-				assert.strictEqual(this.makeParsed(1, 2).evaluate(this.vars), 1);
+				assert.strictEqual(this.parsed.evaluate({a: 1, b: 2}), 1);
 			},
 			"should return the right hand side if the left hand side is null": function() {
-				//assert.strictEqual(this.parsed.evaluate({a: null, b: 2}), 2);
-				assert.strictEqual(this.makeParsed(null, 2).evaluate(this.vars), 2);
+				assert.strictEqual(this.parsed.evaluate({a: null, b: 2}), 2);
 			},
 			"should return the right hand side if the left hand side is undefined": function() {
-				//assert.strictEqual(this.parsed.evaluate({b: 2}), 2);
-				assert.strictEqual(this.makeParsed(undefined, 2).evaluate(this.vars), 2);
+				assert.strictEqual(this.parsed.evaluate({b: 2}), 2);
 			}
 		}
 	}