add test coverage & improve coverage for existing tests

.gitignore

@@ -3,4 +3,5 @@ test/unit/specs.js
 explorations
 node_modules
 .DS_Store
-benchmarks/browser.js
+benchmarks/browser.js
+coverage

gruntfile.js

@@ -48,7 +48,17 @@ module.exports = function (grunt) {
       phantom: {
         options: {
           browsers: ['PhantomJS'],
-          reporters: ['progress']
+          reporters: ['progress', 'coverage'],
+          preprocessors: {
+            'src/**/*.js': ['commonjs', 'coverage'],
+            'test/unit/specs/*.js': ['commonjs']
+          },
+          coverageReporter: {
+            reporters: [
+              { type: 'lcov' },
+              { type: 'text-summary' }
+            ]
+          }
         }
       }
     },
@@ -119,7 +129,7 @@ module.exports = function (grunt) {
   })
 
   grunt.registerTask('unit', ['karma:browsers'])
-  grunt.registerTask('phantom', ['karma:phantom'])
+  grunt.registerTask('cover', ['karma:phantom'])
   grunt.registerTask('bench', ['browserify:bench'])
   grunt.registerTask('watch', ['browserify:watch'])
   grunt.registerTask('build', ['browserify:test', 'browserify:build', 'uglify:build'])

package.json

@@ -31,7 +31,8 @@
     "karma": "^0.12.16",
     "karma-browserify": "^0.2.1",
     "karma-chrome-launcher": "^0.1.4",
-    "karma-commonjs": "0.0.10",
+    "karma-commonjs": "^0.0.10",
+    "karma-coverage": "^0.2.5",
     "karma-firefox-launcher": "^0.1.3",
     "karma-jasmine": "^0.1.5",
     "karma-phantomjs-launcher": "^0.1.4"

src/observe/array-augmentations.js

@@ -51,7 +51,7 @@ var arrayAugmentations = Object.create(Array.prototype)
 
     // link/unlink added/removed elements
     if (inserted) ob.link(inserted, index)
-    if (removed) ob.unlink(removed)
+    if (removed) ob.unlink(removed, index)
 
     // update indices
     if (method !== 'push' && method !== 'pop') {

src/observe/observer.js

@@ -3,6 +3,8 @@ var Emitter = require('../emitter')
 var arrayAugmentations = require('./array-augmentations')
 var objectAugmentations = require('./object-augmentations')
 
+var uid = 0
+
 /**
  * Type enums
  */
@@ -31,6 +33,7 @@ var OBJECT = 1
 
 function Observer (value, type, options) {
   Emitter.call(this, options && options.callbackContext)
+  this.id = ++uid
   this.value = value
   this.type = type
   this.parents = null
@@ -155,11 +158,18 @@ p.observe = function (key, val) {
   var ob = Observer.create(val)
   if (ob) {
     // register self as a parent of the child observer.
-    if (ob.findParent(this) > -1) return
-    (ob.parents || (ob.parents = [])).push({
+    var parents = ob.parents
+    if (!parents) {
+      ob.parents = parents = Object.create(null)
+    }
+    if (parents[this.id]) {
+      _.warn('Observing duplicate key: ' + key)
+      return
+    }
+    parents[this.id] = {
       ob: this,
       key: key
-    })
+    }
   }
 }
 
@@ -172,7 +182,7 @@ p.observe = function (key, val) {
 
 p.unobserve = function (val) {
   if (val && val.$observer) {
-    val.$observer.findParent(this, true)
+    val.$observer.parents[this.id] = null
   }
 }
 
@@ -202,11 +212,6 @@ p.convert = function (key, val) {
       ob.observe(key, newVal)
       ob.emit('set:self', key, newVal)
       ob.propagate('set', key, newVal)
-      if (_.isArray(newVal)) {
-        ob.propagate('set',
-                     key + Observer.pathDelimiter + 'length',
-                     newVal.length)
-      }
     }
   })
 }
@@ -223,14 +228,13 @@ p.convert = function (key, val) {
 p.propagate = function (event, path, val, mutation) {
   this.emit(event, path, val, mutation)
   if (!this.parents) return
-  for (var i = 0, l = this.parents.length; i < l; i++) {
-    var parent = this.parents[i]
-    var ob = parent.ob
+  for (var id in this.parents) {
+    var parent = this.parents[id]
     var key = parent.key
     var parentPath = path
       ? key + Observer.pathDelimiter + path
       : key
-    ob.propagate(event, parentPath, val, mutation)
+    parent.ob.propagate(event, parentPath, val, mutation)
   }
 }
 
@@ -246,32 +250,9 @@ p.updateIndices = function () {
   while (i--) {
     ob = arr[i] && arr[i].$observer
     if (ob) {
-      var j = ob.findParent(this)
-      ob.parents[j].key = i
-    }
-  }
-}
-
-/**
- * Find a parent option object
- *
- * @param {Observer} parent
- * @param {Boolean} [remove] - whether to remove the parent
- * @return {Number} - index of parent
- */
-
-p.findParent = function (parent, remove) {
-  var parents = this.parents
-  if (!parents) return -1
-  var i = parents.length
-  while (i--) {
-    var p = parents[i]
-    if (p.ob === parent) {
-      if (remove) parents.splice(i, 1)
-      return i
+      ob.parents[this.id].key = i
     }
   }
-  return -1
 }
 
 module.exports = Observer

src/parse/path.js

@@ -159,10 +159,7 @@ function parsePath (path) {
     }
   }
 
-  function maybeUnescapeQuote() {
-    if (index >= path.length) {
-      return
-    }
+  function maybeUnescapeQuote () {
     var nextChar = path[index + 1]
     if ((mode === 'inSingleQuote' && nextChar === "'") ||
         (mode === 'inDoubleQuote' && nextChar === '"')) {
@@ -177,7 +174,7 @@ function parsePath (path) {
     index++
     c = path[index]
 
-    if (c === '\\' && maybeUnescapeQuote(mode)) {
+    if (c === '\\' && maybeUnescapeQuote()) {
       continue
     }
 
@@ -198,8 +195,6 @@ function parsePath (path) {
       return keys
     }
   }
-
-  return // parse error
 }
 
 /**
@@ -320,7 +315,7 @@ exports.set = function (obj, path, val) {
     path = exports.parse(path)
   }
   if (!path) {
-    return
+    return false
   }
   for (var i = 0, l = path.length - 1; i < l; i++) {
     if (!obj || typeof obj !== 'object') {

src/parse/template.js

@@ -1,3 +1,5 @@
+/* global DocumentFragment */
+
 var Cache = require('../cache')
 var templateCache = new Cache(100)
 
@@ -93,7 +95,7 @@ function nodeToFragment (node) {
   var tag = node.tagName
   // if its a template tag and the browser supports it,
   // its content is already a document fragment.
-  if (tag === 'TEMPLATE' && node.content) {
+  if (tag === 'TEMPLATE' && node.content instanceof DocumentFragment) {
     return node.content
   }
   return tag === 'SCRIPT'
@@ -119,7 +121,7 @@ exports.parse = function (template) {
   var node, frag
 
   // if the template is already a document fragment -- do nothing
-  if (template instanceof window.DocumentFragment) {
+  if (template instanceof DocumentFragment) {
     return template
   }
 

src/util/debug.js

@@ -14,26 +14,26 @@ function enableDebug () {
   /**
    * Log a message.
    *
-   * @param {String} msg
+   * @param {*...}
    */
 
-  exports.log = function (msg) {
+  exports.log = function () {
     if (hasConsole && config.debug) {
-      console.log(msg)
+      console.log.apply(console, arguments)
     }
   }
 
   /**
    * We've got a problem here.
    *
-   * @param {String} msg
+   * @param {*...}
    */
 
-  exports.warn = function (msg) {
+  exports.warn = function () {
     if (hasConsole && !config.silent) {
-      console.warn(msg)
+      console.warn.apply(console, arguments)
       if (config.debug && console.trace) {
-        console.trace(msg)
+        console.trace()
       }
     }
   }

test/unit/specs/directive_parser_spec.js

@@ -17,18 +17,22 @@ describe('Directive Parser', function () {
     expect(res[0].raw).toBe('arg:exp')
   })
 
-  it('arg : exp | abc', function () {
-    var res = parse(' arg : exp | abc de')
+  // filters
+  it('arg : exp | abc de | bcd', function () {
+    var res = parse(' arg : exp | abc de | bcd')
     expect(res.length).toBe(1)
     expect(res[0].expression).toBe('exp')
     expect(res[0].arg).toBe('arg')
-    expect(res[0].raw).toBe('arg : exp | abc de')
-    expect(res[0].filters.length).toBe(1)
+    expect(res[0].raw).toBe('arg : exp | abc de | bcd')
+    expect(res[0].filters.length).toBe(2)
     expect(res[0].filters[0].name).toBe('abc')
     expect(res[0].filters[0].args.length).toBe(1)
     expect(res[0].filters[0].args[0]).toBe('de')
+    expect(res[0].filters[1].name).toBe('bcd')
+    expect(res[0].filters[1].args).toBeNull()
   })
 
+  // double pipe
   it('a || b | c', function () {
     var res = parse('a || b | c')
     expect(res.length).toBe(1)
@@ -39,13 +43,15 @@ describe('Directive Parser', function () {
     expect(res[0].filters[0].args).toBeNull()
   })
 
-  it('a ? b : c', function () {
-    var res = parse('a ? b : c')
+  // single quote + boolean
+  it('a ? \'b\' : c', function () {
+    var res = parse('a ? \'b\' : c')
     expect(res.length).toBe(1)
-    expect(res[0].expression).toBe('a ? b : c')
+    expect(res[0].expression).toBe('a ? \'b\' : c')
     expect(res[0].filters).toBeUndefined()
   })
 
+  // double quote + boolean
   it('"a:b:c||d|e|f" || d ? a : b', function () {
     var res = parse('"a:b:c||d|e|f" || d ? a : b')
     expect(res.length).toBe(1)
@@ -54,6 +60,7 @@ describe('Directive Parser', function () {
     expect(res[0].arg).toBeUndefined()
   })
 
+  // multiple simple clause
   it('a, b, c', function () {
     var res = parse('a, b, c')
     expect(res.length).toBe(3)
@@ -62,21 +69,27 @@ describe('Directive Parser', function () {
     expect(res[2].expression).toBe('c')
   })
 
-  it('a:b | c, d:e | f, g:h | i', function () {
-    var res = parse('a:b | c, d:e | f, g:h | i')
+  // multiple complex clause
+  it('a:b | c | j, d:e | f | k l, g:h | i', function () {
+    var res = parse('a:b | c | j, d:e | f | k l, g:h | i')
     expect(res.length).toBe(3)
 
     expect(res[0].arg).toBe('a')
     expect(res[0].expression).toBe('b')
-    expect(res[0].filters.length).toBe(1)
+    expect(res[0].filters.length).toBe(2)
     expect(res[0].filters[0].name).toBe('c')
     expect(res[0].filters[0].args).toBeNull()
+    expect(res[0].filters[1].name).toBe('j')
+    expect(res[0].filters[1].args).toBeNull()
 
     expect(res[1].arg).toBe('d')
     expect(res[1].expression).toBe('e')
-    expect(res[1].filters.length).toBe(1)
+    expect(res[1].filters.length).toBe(2)
     expect(res[1].filters[0].name).toBe('f')
     expect(res[1].filters[0].args).toBeNull()
+    expect(res[1].filters[1].name).toBe('k')
+    expect(res[1].filters[1].args.length).toBe(1)
+    expect(res[1].filters[1].args[0]).toBe('l')
 
     expect(res[2].arg).toBe('g')
     expect(res[2].expression).toBe('h')
@@ -85,6 +98,7 @@ describe('Directive Parser', function () {
     expect(res[2].filters[0].args).toBeNull()
   })
 
+  // super complex
   it('click:test(c.indexOf(d,f),"e,f"), input: d || [e,f], ok:{a:1,b:2}', function () {
     var res = parse('click:test(c.indexOf(d,f),"e,f"), input: d || [e,f], ok:{a:1,b:2}')
     expect(res.length).toBe(3)

test/unit/specs/expression_parser_spec.js

@@ -1,4 +1,5 @@
 var expParser = require('../../../src/parse/expression')
+var _ = require('../../../src/util')
 
 function assertExp (testCase) {
   var res = expParser.parse(testCase.exp)
@@ -217,9 +218,40 @@ describe('Expression Parser', function () {
   })
 
   it('cache', function () {
-    var fn1 = expParser.parse('a + b')
-    var fn2 = expParser.parse('a + b')
-    expect(fn1).toBe(fn2)
+    var res1 = expParser.parse('a + b')
+    var res2 = expParser.parse('a + b')
+    expect(res1).toBe(res2)
   })
 
-})
+  describe('invalid expression', function () {
+    
+    beforeEach(function () {
+      spyOn(_, 'warn')
+    })
+
+    it('should warn on invalid expression', function () {
+      var res = expParser.parse('a--b"ffff')
+      expect(_.warn).toHaveBeenCalled()
+    })
+
+    if (leftHandThrows()) {
+      it('should warn on invalid left hand expression for setter', function () {
+        var res = expParser.parse('a+b', true)
+        expect(_.warn).toHaveBeenCalled()
+      })
+    }
+  })
+})
+
+/**
+ * check if creating a new Function with invalid left-hand
+ * assignment would throw
+ */
+
+function leftHandThrows () {
+  try {
+    var fn = new Function('a + b = 1')
+  } catch (e) {
+    return true
+  }
+}

test/unit/specs/observer_spec.js

@@ -3,6 +3,7 @@
  */
 
 var Observer = require('../../../src/observe/observer')
+var _ = require('../../../src/util')
 // internal emitter has fixed 3 arguments
 // so we need to fill up the assetions with undefined
 var u = undefined
@@ -93,6 +94,17 @@ describe('Observer', function () {
     expect(obj.b).toBe(234)
   })
 
+  it('warn duplicate value', function () {
+    spyOn(_, 'warn')
+    var obj = {
+      a: { b: 123 },
+      b: null
+    }
+    var ob = Observer.create(obj)
+    obj.b = obj.a
+    expect(_.warn).toHaveBeenCalled()
+  })
+
   it('array get', function () {
 
     Observer.emitGet = true
@@ -274,6 +286,10 @@ describe('Observer', function () {
     var ob = Observer.create(obj)
     ob.on('add', spy)
 
+    // ignore existing keys
+    obj.$add('a', 123)
+    expect(spy.callCount).toBe(0)
+
     // add event
     var add = {d:2}
     obj.a.$add('c', add)
@@ -290,6 +306,10 @@ describe('Observer', function () {
     var ob = Observer.create(obj)
     ob.on('delete', spy)
 
+    // ignore non-present key
+    obj.$delete('c')
+    expect(spy.callCount).toBe(0)
+
     obj.a.$delete('b')
     expect(spy).toHaveBeenCalledWith('a.b', u, u)
   })
@@ -318,6 +338,16 @@ describe('Observer', function () {
     expect(spy).toHaveBeenCalledWith('1.a', 4, u)
   })
 
+  it('array.$set with out of bound length', function () {
+    var arr = [{a:1}, {a:2}]
+    var ob = Observer.create(arr)
+    var inserted = {a:3}
+    arr.$set(3, inserted)
+    expect(arr.length).toBe(4)
+    expect(arr[2]).toBeUndefined()
+    expect(arr[3]).toBe(inserted)
+  })
+
   it('array.$remove', function () {
     var arr = [{a:1}, {a:2}]
     var ob = Observer.create(arr)
@@ -339,4 +369,25 @@ describe('Observer', function () {
     expect(spy).toHaveBeenCalledWith('0.a', 3, u)
   })
 
+  it('array.$remove object', function () {
+    var arr = [{a:1}, {a:2}]
+    var ob = Observer.create(arr)
+    ob.on('mutate', spy)
+    var removed = arr.$remove(arr[0])
+
+    expect(spy.mostRecentCall.args[0]).toBe('')
+    expect(spy.mostRecentCall.args[1]).toBe(arr)
+    var mutation = spy.mostRecentCall.args[2]
+    expect(mutation).toBeDefined()
+    expect(mutation.method).toBe('splice')
+    expect(mutation.index).toBe(0)
+    expect(mutation.removed.length).toBe(1)
+    expect(mutation.inserted.length).toBe(0)
+    expect(mutation.removed[0]).toBe(removed)
+
+    ob.on('set', spy)
+    arr[0].a = 3
+    expect(spy).toHaveBeenCalledWith('0.a', 3, u)
+  })
+
 })

test/unit/specs/path_parser_spec.js

@@ -92,7 +92,7 @@ describe('Path', function () {
       }
     }
     expect(Path.getFromArray(obj, path)).toBe(123)
-    expect(Path.getFromArray(obj, ['a','c'])).toBeUndefined()
+    expect(Path.getFromArray(obj, ['a','c','d'])).toBeUndefined()
   })
 
   it('get from observer delimited path', function () {
@@ -122,11 +122,17 @@ describe('Path', function () {
   })
 
   it('set fail', function () {
-    var path = 'a.b.c'
     var obj = {
       a: null
     }
-    var res = Path.set(obj, path, 12345)
+    var res = Path.set(obj, 'a.b.c', 12345)
+    expect(res).toBe(false)
+    res = Path.set(obj, 'a.b', 12345)
+    expect(res).toBe(false)
+  })
+
+  it('set invalid', function () {
+    var res = Path.set({}, 'ab[c]d', 123)
     expect(res).toBe(false)
   })
 

test/unit/specs/template.js

@@ -10,14 +10,15 @@ describe('Template Parser', function () {
     expect(res).toBe(frag)
   })
 
-  // only test template node if it works in the browser being tested.
-  var templateNode = document.createElement('template')
-  if (templateNode.content) {
-    it('should return content if argument is a valid template node', function () {
-      var res = parse(templateNode)
-      expect(res).toBe(templateNode.content)
-    })
-  }
+  it('should return content if argument is a valid template node', function () {
+    var templateNode = document.createElement('template')
+    if (!templateNode.content) {
+      // mock the content 
+      templateNode.content = document.createDocumentFragment()
+    }
+    var res = parse(templateNode)
+    expect(res).toBe(templateNode.content)
+  })
 
   it('should parse if argument is a template string', function () {
     var res = parse(testString)
@@ -26,6 +27,13 @@ describe('Template Parser', function () {
     expect(res.querySelector('.test').textContent).toBe('world')
   })
 
+  it('should work if the template string doesn\'t contain tags', function () {
+    var res = parse('hello!')
+    expect(res instanceof DocumentFragment).toBeTruthy()
+    expect(res.childNodes.length).toBe(1)
+    expect(res.firstChild.nodeType).toBe(3) // Text node
+  })
+
   it('should parse textContent if argument is a script node', function () {
     var node = document.createElement('script')
     node.textContent = testString

test/unit/specs/util_spec.js

@@ -163,6 +163,14 @@ describe('Util', function () {
         expect(child.nextSibling).toBe(target)
       })
 
+      it('after with sibling', function () {
+        var sibling = div()
+        parent.appendChild(sibling)
+        _.after(target, child)
+        expect(target.parentNode).toBe(parent)
+        expect(child.nextSibling).toBe(target)
+      })
+
       it('remove', function () {
         _.remove(child)
         expect(child.parentNode).toBeNull()
@@ -175,6 +183,13 @@ describe('Util', function () {
         expect(parent.firstChild).toBe(target)
       })
 
+      it('prepend to empty node', function () {
+        parent.removeChild(child)
+        _.prepend(target, parent)
+        expect(target.parentNode).toBe(parent)
+        expect(parent.firstChild).toBe(target)
+      })
+
       it('copyAttributes', function () {
         parent.setAttribute('test1', 1)
         parent.setAttribute('test2', 2)
@@ -183,9 +198,56 @@ describe('Util', function () {
         expect(target.getAttribute('test1')).toBe('1')
         expect(target.getAttribute('test2')).toBe('2')
       })
-
     })
+  }
+
+  if (typeof console !== undefined) {
+
+    describe('Debug', function () {
 
+      beforeEach(function () {
+        spyOn(console, 'log')
+        spyOn(console, 'warn')
+        if (console.trace) {
+          spyOn(console, 'trace')
+        }
+      })
+      
+      it('log when debug is true', function () {
+        config.debug = true
+        _.log('hello', 'world')
+        expect(console.log).toHaveBeenCalledWith('hello', 'world')
+      })
+
+      it('not log when debug is false', function () {
+        config.debug = false
+        _.log('bye', 'world')
+        expect(console.log.callCount).toBe(0)
+      })
+
+      it('warn when silent is false', function () {
+        config.silent = false
+        _.warn('oops', 'ops')
+        expect(console.warn).toHaveBeenCalledWith('oops', 'ops')
+      })
+
+      it('not warn when silent is ture', function () {
+        config.silent = true
+        _.warn('oops', 'ops')
+        expect(console.warn.callCount).toBe(0)
+      })
+
+      if (console.trace) {
+        it('trace when not silent and debugging', function () {
+          config.debug = true
+          config.silent = false
+          _.warn('haha')
+          expect(console.trace).toHaveBeenCalled()
+          config.debug = false
+          config.silent = true
+        })
+      }
+    })
   }
 
   describe('Option merging', function () {