From 3c5ac39622ae06631d468c5bde7f4daa12ebddb0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jos=C3=A9=20Manuel=20Aguirre?= <jmaguirrei@gmail.com>
Date: Fri, 26 Oct 2018 10:25:10 -0300
Subject: [PATCH] PR to improve tests by requiring callback fn only for async
 tests

---
 Section 6/Adding Unit Tests/test/index.js | 54 +++++++++++++++++++----
 Section 6/Adding Unit Tests/test/unit.js  | 21 +++++----
 2 files changed, 58 insertions(+), 17 deletions(-)

diff --git a/Section 6/Adding Unit Tests/test/index.js b/Section 6/Adding Unit Tests/test/index.js
index 2bd96f1..20c9c4b 100644
--- a/Section 6/Adding Unit Tests/test/index.js	
+++ b/Section 6/Adding Unit Tests/test/index.js	
@@ -12,6 +12,24 @@ _app.tests = {};
 // Dependencies
 _app.tests.unit = require('./unit');
 
+
+/*
+ * PR for final Exam
+ * getFunctionParams Helper
+ * Allows to improve the way test are defined
+ * by making the done() callback only necessary for async tests.
+ */
+
+const STRIP_COMMENTS = /((\/\/.*$)|(\/\*[\s\S]*?\*\/))/mg;
+const ARGUMENT_NAMES = /([^\s,]+)/g;
+
+_app.getFunctionParams = function (func) {
+  const fnStr = func.toString().replace(STRIP_COMMENTS, '');
+  let result = fnStr.slice(fnStr.indexOf('(') + 1, fnStr.indexOf(')')).match(ARGUMENT_NAMES);
+  if (result === null) result = [];
+  return result;
+};
+
 // Count all the tests
 _app.countTests = function(){
   var counter = 0;
@@ -42,17 +60,35 @@ _app.runTests = function(){
             (function(){
               var tmpTestName = testName;
               var testValue = subTests[testName];
+
+              /*
+               * PR for final Exam
+               * Callback function "done" extracted because it can be called in 2 ways
+               * As a callback for async test and as a direct call for sync tests
+               */
+              function done() {
+                // If it calls back without throwing, then it succeeded, so log it in green
+                console.log('\x1b[32m%s\x1b[0m',tmpTestName);
+                counter++;
+                successes++;
+                if(counter == limit){
+                  _app.produceTestReport(limit,successes,errors);
+                }
+              }
               // Call the test
               try{
-                testValue(function(){
-                  // If it calls back without throwing, then it succeeded, so log it in green
-                  console.log('\x1b[32m%s\x1b[0m',tmpTestName);
-                  counter++;
-                  successes++;
-                  if(counter == limit){
-                    _app.produceTestReport(limit,successes,errors);
-                  }
-                });
+
+                /*
+                 * PR for final Exam
+                 * 2 ways to call the testValue function
+                 */
+                const doesTestNeedCallback = _app.getFunctionParams(testValue);
+                if (doesTestNeedCallback) {
+                  testValue(done);
+                } else {
+                  testValue();
+                  done();
+                }
               } catch(e){
                 // If it throws, then it failed, so capture the error thrown and log it in red
                 errors.push({
diff --git a/Section 6/Adding Unit Tests/test/unit.js b/Section 6/Adding Unit Tests/test/unit.js
index cb851d9..d24ee69 100644
--- a/Section 6/Adding Unit Tests/test/unit.js	
+++ b/Section 6/Adding Unit Tests/test/unit.js	
@@ -13,25 +13,30 @@ var assert = require('assert');
 var unit = {};
 
 
+/*
+ * PR for final Exam
+ * All sync test does not require callback (done) function
+ */
+
 // Assert that the getANumber function is returning a number
-unit['helpers.getANumber should return a number'] = function(done){
+unit['helpers.getANumber should return a number'] = function(/* done */){
   var val = helpers.getANumber();
   assert.equal(typeof(val), 'number');
-  done();
+  // done();
 };
 
 // Assert that the getANumber function is returning 1
-unit['helpers.getANumber should return 1'] = function(done){
+unit['helpers.getANumber should return 1'] = function(/* done */){
   var val = helpers.getANumber();
   assert.equal(val, 1);
-  done();
+  // done();
 };
 
 // Assert that the getANumber function is returning 2
-unit['helpers.getNumberOne should return 2'] = function(done){
+unit['helpers.getNumberOne should return 2'] = function(/* done */){
   var val = helpers.getANumber();
   assert.equal(val, 2);
-  done();
+  // done();
 };
 
 // Logs.list should callback an array and a false error
@@ -55,10 +60,10 @@ unit['logs.truncate should not throw if the logId does not exist, should callbac
 };
 
 // exampleDebuggingProblem.init should not throw (but it does)
-unit['exampleDebuggingProblem.init should not throw when called'] = function(done){
+unit['exampleDebuggingProblem.init should not throw when called'] = function(/* done */){
   assert.doesNotThrow(function(){
     exampleDebuggingProblem.init();
-    done();
+    // done();
   },TypeError);
 };