Achieve 100% test coverage (#349)

* test: Router module integration

test: findMyWay.findRoute should also trows an error when wildcard is not the last character

test: does not find the route if maxParamLength is exceeded

refactor: make integration test more compliant with the existing code base

test: cover uncovered constrainer.js test cases

test: safeDecodeURIComponent should replace %3x to null for every x that is not a valid lowchar

test: SemVerStore version should be a string

test: httpMethodStrategy storage handles set and get operations correctly

test: case insensitive static routes of level 1 for FindMyWay.findRoute

test: findRoute normalizes wildcard patterns to require leading slash

refactor: make tests more consistants with the existing

test: should return null if no wildchar child

Date:      Tue Feb 20 14:10:18 2024 +0100
Achieves 100% test coverage

* fix: add proxyquire to dev dependencies

* test: cover uncovered branches in index.js

* fix: revert 'if (nextCharCode === 58)' stmt to one line

* fix: remove useless parametric node kind check

* fix: if it is a wildchar node, it has a child

* fix: a wildcard route created via the router api necessarily has a pattern with a leading slash

* fix: non-custom strategy error only happen when user dont use the appropriate api

* fix: there is no way found route params is not an array, expect if user break internals

* test: Constrainer.noteUsage

* Cannot derive constraints without active strategies.

* test: should derive multiple async constraints

* test: Major version must be a numeric value

* test: SemVerStore.maxMajor should increase automatically

* test: SemVerStore.maxPatches should increase automatically

* refactor: httpMethods module should be a source of truth

* refactor: make consistent use of fmw.on

* fix: istanbul should ignore the complete function

* test: remove 90 min limit for code coverage

* fix: when no args is passed, undefined is default value

* fix: add new line at the end of .taprc

* fix: readd removed comment indicating static route

Author: jean-michelet

.taprc

@@ -1,7 +1,3 @@
 ts: false
 jsx: false
 flow: false
-branches: 90
-functions: 90
-lines: 90
-statements: 90

index.js

@@ -344,7 +344,6 @@ Router.prototype.findRoute = function findNode (method, path, constraints = {})
         const isRegexParam = charCode === 40
         const isStaticPart = charCode === 45 || charCode === 46
         const isEndOfNode = charCode === 47 || j === pattern.length
-
         if (isRegexParam || isStaticPart || isEndOfNode) {
           const paramName = pattern.slice(lastParamStartIndex, j)
           params.push(paramName)
@@ -407,9 +406,7 @@ Router.prototype.findRoute = function findNode (method, path, constraints = {})
       // add the wildcard parameter
       params.push('*')
       currentNode = currentNode.getWildcardChild()
-      if (currentNode === null) {
-        return null
-      }
+
       parentNodePathIndex = i + 1
 
       if (i !== pattern.length - 1) {
@@ -422,10 +419,6 @@ Router.prototype.findRoute = function findNode (method, path, constraints = {})
     pattern = pattern.toLowerCase()
   }
 
-  if (pattern === '*') {
-    pattern = '/*'
-  }
-
   for (const existRoute of this.routes) {
     const routeConstraints = existRoute.opts.constraints || {}
     if (
@@ -436,7 +429,7 @@ Router.prototype.findRoute = function findNode (method, path, constraints = {})
       return {
         handler: existRoute.handler,
         store: existRoute.store,
-        params: existRoute.params || []
+        params: existRoute.params
       }
     }
   }
@@ -642,37 +635,36 @@ Router.prototype.find = function find (method, path, derivedConstraints) {
       continue
     }
 
-    if (currentNode.kind === NODE_TYPES.PARAMETRIC) {
-      let paramEndIndex = originPath.indexOf('/', pathIndex)
-      if (paramEndIndex === -1) {
-        paramEndIndex = pathLen
-      }
+    // parametric node
+    let paramEndIndex = originPath.indexOf('/', pathIndex)
+    if (paramEndIndex === -1) {
+      paramEndIndex = pathLen
+    }
 
-      let param = originPath.slice(pathIndex, paramEndIndex)
-      if (shouldDecodeParam) {
-        param = safeDecodeURIComponent(param)
-      }
+    let param = originPath.slice(pathIndex, paramEndIndex)
+    if (shouldDecodeParam) {
+      param = safeDecodeURIComponent(param)
+    }
 
-      if (currentNode.isRegex) {
-        const matchedParameters = currentNode.regex.exec(param)
-        if (matchedParameters === null) continue
+    if (currentNode.isRegex) {
+      const matchedParameters = currentNode.regex.exec(param)
+      if (matchedParameters === null) continue
 
-        for (let i = 1; i < matchedParameters.length; i++) {
-          const matchedParam = matchedParameters[i]
-          if (matchedParam.length > maxParamLength) {
-            return null
-          }
-          params.push(matchedParam)
-        }
-      } else {
-        if (param.length > maxParamLength) {
+      for (let i = 1; i < matchedParameters.length; i++) {
+        const matchedParam = matchedParameters[i]
+        if (matchedParam.length > maxParamLength) {
           return null
         }
-        params.push(param)
+        params.push(matchedParam)
       }
-
-      pathIndex = paramEndIndex
+    } else {
+      if (param.length > maxParamLength) {
+        return null
+      }
+      params.push(param)
     }
+
+    pathIndex = paramEndIndex
   }
 }
 
@@ -742,8 +734,6 @@ for (const i in httpMethods) {
   const m = httpMethods[i]
   const methodName = m.toLowerCase()
 
-  if (Router.prototype[methodName]) throw new Error('Method already exists: ' + methodName)
-
   Router.prototype[methodName] = function (path, handler, store) {
     return this.on(m, path, handler, store)
   }

lib/constrainer.js

@@ -153,10 +153,8 @@ class Constrainer {
       if (!strategy.isCustom) {
         if (key === 'version') {
           lines.push('   version: req.headers[\'accept-version\'],')
-        } else if (key === 'host') {
-          lines.push('   host: req.headers.host || req.headers[\':authority\'],')
         } else {
-          throw new Error('unknown non-custom strategy for compiling constraint derivation function')
+          lines.push('   host: req.headers.host || req.headers[\':authority\'],')
         }
       } else {
         lines.push(`  ${strategy.name}: this.strategies.${key}.deriveConstraint(req, ctx),`)

lib/node.js

@@ -129,10 +129,7 @@ class StaticNode extends ParentNode {
   }
 
   getWildcardChild () {
-    if (this.wildcardChild) {
-      return this.wildcardChild
-    }
-    return null
+    return this.wildcardChild
   }
 
   createWildcardChild () {

lib/strategies/accept-version.js

@@ -20,7 +20,11 @@ SemVerStore.prototype.set = function (version, store) {
   }
   let [major, minor, patch] = version.split('.')
 
-  major = Number(major) || 0
+  if (isNaN(major)) {
+    throw new TypeError('Major version must be a numeric value')
+  }
+
+  major = Number(major)
   minor = Number(minor) || 0
   patch = Number(patch) || 0
 
@@ -38,7 +42,7 @@ SemVerStore.prototype.set = function (version, store) {
     this.store[`${major}.x.x`] = store
   }
 
-  if (patch >= (this.store[`${major}.${minor}`] || 0)) {
+  if (patch >= (this.maxPatches[`${major}.${minor}`] || 0)) {
     this.maxPatches[`${major}.${minor}`] = patch
     this.store[`${major}.${minor}.x`] = store
   }

lib/strategies/http-method.js

@@ -9,9 +9,6 @@ module.exports = {
       set: (type, store) => { handlers[type] = store }
     }
   },
-  deriveConstraint: (req) => {
-    /* istanbul ignore next */
-    return req.method
-  },
+  deriveConstraint: /* istanbul ignore next */ (req) => req.method,
   mustMatchWhenDerived: true
 }

package.json

@@ -40,6 +40,7 @@
     "chalk": "^4.1.2",
     "inquirer": "^8.2.4",
     "pre-commit": "^1.2.2",
+    "proxyquire": "^2.1.3",
     "rfdc": "^1.3.0",
     "simple-git": "^3.7.1",
     "standard": "^14.3.4",

test/constraint.custom.async.test.js

@@ -3,6 +3,7 @@
 const t = require('tap')
 const test = t.test
 const FindMyWay = require('..')
+const rfdc = require('rfdc')({ proto: true })
 
 const customHeaderConstraint = {
   name: 'requestedBy',
@@ -23,11 +24,14 @@ const customHeaderConstraint = {
   }
 }
 
-test('should derive async constraint', t => {
+test('should derive multiple async constraints', t => {
   t.plan(2)
 
-  const router = FindMyWay({ constraints: { requestedBy: customHeaderConstraint } })
-  router.on('GET', '/', { constraints: { requestedBy: 'node' } }, () => 'asyncHandler')
+  const customHeaderConstraint2 = rfdc(customHeaderConstraint)
+  customHeaderConstraint2.name = 'requestedBy2'
+
+  const router = FindMyWay({ constraints: { requestedBy: customHeaderConstraint, requestedBy2: customHeaderConstraint2 } })
+  router.on('GET', '/', { constraints: { requestedBy: 'node', requestedBy2: 'node' } }, () => 'asyncHandler')
 
   router.lookup(
     {