Don't break up as URL for autocomplete if location bar text isn't a URL

app/common/lib/siteSuggestions.js

@@ -102,8 +102,9 @@ const tokenizeInput = (data) => {
     }
   }
 
-  if (url && isUrl(url)) {
-    const parsedUrl = urlParse(url.toLowerCase())
+  const parsedUrl = typeof url === 'string' && isUrl(url) && urlParse(url.toLowerCase())
+
+  if (parsedUrl && (parsedUrl.hash || parsedUrl.host || parsedUrl.pathname || parsedUrl.query || parsedUrl.protocol)) {
     if (parsedUrl.hash) {
       parts.push(parsedUrl.hash.slice(1))
     }

js/muonTest.entry.js

@@ -1,81 +1,49 @@
-const urlParse = require('../app/common/urlParse')
-const urlUtil = require('./lib/urlutil')
-const suggestion = require('../app/common/lib/suggestion')
 const assert = require('assert')
-
-const assertEqual = (actual, expected, name) => {
-  const elem = document.createElement('div')
-  elem.id = name
-  elem.innerText = 'fail'
-
-  try {
-    assert.deepEqual(actual, expected)
-    elem.innerText = 'success'
-  } catch (e) {
-    elem.innerText = JSON.stringify(actual)
+const tests = require('../test/muon-native/').run
+
+const testRunner = (successKey) => {
+  const failures = []
+  let assertCount = 0
+
+  const assertWrap = (assertType, assertion, failText) => {
+    assertCount++
+    try {
+      assertion()
+    } catch (e) {
+      failures.push({ assertCount, assertType, failText })
+    }
   }
-  document.body.appendChild(elem)
-}
-
-const defaultParsedUrl = {
-  hash: '',
-  host: '',
-  hostname: '',
-  href: '',
-  origin: '',
-  path: '/',
-  pathname: '/',
-  port: '',
-  protocol: 'http:',
-  query: '',
-  search: ''
-}
-
-const runTests = () => {
-  assertEqual(urlParse('http://bing.com'),
-    Object.assign(defaultParsedUrl, {
-      host: 'bing.com',
-      hostname: 'bing.com',
-      origin: 'http://bing.com/',
-      href: 'http://bing.com/'
-    }), 'urlParseSimple')
 
-  assertEqual(urlParse('https://brave.com:333/test?abc=123&def#fff'),
-    {
-      host: 'brave.com:333',
-      hostname: 'brave.com',
-      origin: 'https://brave.com:333/',
-      protocol: 'https:',
-      port: '333',
-      hash: '#fff',
-      pathname: '/test',
-      path: '/test?abc=123&def',
-      search: '?abc=123&def',
-      query: 'abc=123&def',
-      href: 'https://brave.com:333/test?abc=123&def#fff'
-    }, 'urlParseComplex')
-
-  assertEqual(urlParse('http://brave.com%60x.code-fu.org/'),
-    Object.assign(defaultParsedUrl, {
-      host: 'brave.com%60x.code-fu.org',
-      hostname: 'brave.com%60x.code-fu.org',
-      href: 'http://brave.com%60x.code-fu.org/',
-      origin: 'http://brave.com%60x.code-fu.org/'
-    }), 'urlParseIssue10270')
+  const done = () => {
+    const elem = document.createElement('div')
+    // selector must match the pattern used in muonTest.js
+    elem.id = successKey.replace(/[^a-zA-Z0-9_-]/g, '_')
+    // passFail is the critical element, it has to contain either 'PASS' or 'FAIL'
+    const passFail = document.createElement('span')
+    passFail.className = 'passFail'
+    passFail.innerText = 'PASS'
+    const desc = document.createElement('span')
+    desc.innerText = ` ${successKey}`
+    const failure = document.createElement('div')
+    failure.className = 'failure'
+    if (failures.length) {
+      passFail.innerText = 'FAIL'
+      for (let fail of failures) {
+        failure.innerText += `#${fail.assertCount} ${fail.assertType}: ${fail.failText}\n`
+      }
+    }
+    elem.appendChild(passFail)
+    elem.appendChild(desc)
+    elem.appendChild(failure)
+    document.body.appendChild(elem)
+  }
 
-  assertEqual(urlUtil.getOrigin('http://www.brave.com/foo'), 'http://www.brave.com', 'getOriginSimple')
-  assertEqual(urlUtil.getOrigin('file:///aaa'), 'file:///', 'getOriginFile')
-  assertEqual(urlUtil.getOrigin('http://brave.com:333/foo'), 'http://brave.com:333', 'getOriginWithPort')
-  assertEqual(urlUtil.getOrigin('http://127.0.0.1:443/?test=1#abc'), 'http://127.0.0.1:443', 'getOriginIP')
-  assertEqual(urlUtil.getOrigin('about:preferences#abc'), 'about:preferences', 'getOriginAbout')
-  assertEqual(urlUtil.getOrigin('http://http/test'), 'http://http', 'getOriginSchemeHost')
-  assertEqual(urlUtil.getOrigin(''), null, 'getOriginNull')
-  assertEqual(urlUtil.getOrigin('abc'), null, 'getOriginInvalid')
-  console.log(suggestion.isSimpleDomainNameValue('http://http/test'))
-  assertEqual(suggestion.isSimpleDomainNameValue('http://test.com') &&
-    suggestion.isSimpleDomainNameValue('http://test.com/') &&
-    suggestion.isSimpleDomainNameValue('http://test.com#') &&
-    !suggestion.isSimpleDomainNameValue('http://test.com/test'), true, 'suggestionSimpleCheck')
+  return [ 'ok', 'equal', 'deepEqual', 'strictEqual' ].reduce((wrappedAssert, fn) => {
+    wrappedAssert[fn] = (actual, expected) => {
+      assertWrap(fn, () => { assert[fn](actual, expected) }, actual)
+    }
+    return wrappedAssert
+  }, { done })
 }
 
-runTests()
+tests(testRunner)

package.json

@@ -177,6 +177,7 @@
     "less": "^2.5.3",
     "less-loader": "^2.2.1",
     "level": "^2.1.1",
+    "lolex": "~1.3.2",
     "mkdirp": "^0.5.1",
     "mocha": "^2.3.4",
     "mockery": "^2.1.0",

test/lib/brave.js

@@ -365,7 +365,7 @@ var exports = {
     })
 
     this.app.client.addCommand('waitForTextValue', function (selector, text) {
-      logVerbose('waitForSelectedText("' + selector + '", "' + text + '")')
+      logVerbose('waitForTextValue("' + selector + '", "' + text + '")')
       return this
         .waitForVisible(selector)
         .waitUntil(function () {

test/misc-components/muonTest.js

@@ -3,6 +3,7 @@
 const Brave = require('../lib/brave')
 
 const testUrl = 'chrome-extension://mnojpmjdmbbfmejpflffifhffcmidifd/muon-tests.html'
+const testCases = require('../muon-native').collect()
 
 function * setup (client) {
   yield client.waitForUrl(Brave.newTabUrl).waitForBrowserWindow()
@@ -16,24 +17,46 @@ describe('muon tests', function () {
       .tabByIndex(0)
       .url(testUrl)
   })
-  it('muon.url.parse', function * () {
-    yield this.app.client
-      .waitForTextValue('#urlParseSimple', 'success')
-      .waitForTextValue('#urlParseComplex', 'success')
-      .waitForTextValue('#urlParseIssue10270', 'success')
-  })
-  it('urlUtil.getOrigin', function * () {
-    yield this.app.client
-      .waitForTextValue('#getOriginSimple', 'success')
-      .waitForTextValue('#getOriginFile', 'success')
-      .waitForTextValue('#getOriginWithPort', 'success')
-      .waitForTextValue('#getOriginIP', 'success')
-      .waitForTextValue('#getOriginAbout', 'success')
-      .waitForTextValue('#getOriginNull', 'success')
-      .waitForTextValue('#getOriginInvalid', 'success')
-  })
-  it('suggestion', function * () {
-    yield this.app.client
-      .waitForTextValue('#suggestionSimpleCheck', 'success')
-  })
+
+  const makeKey = (key, ext) => `${key} → ${ext}`
+  // selector must match the pattern used in muonTest.entry.js
+  const makeSelector = (key) => `#${key.replace(/[^a-zA-Z0-9_-]/g, '_')}`
+
+  const executeTests = (name, successKey, tests) => {
+    const runnableTests = Object.keys(tests).filter((k) => typeof tests[k] === 'function')
+    if (runnableTests.length) {
+      // actual tests, with a `function`
+      for (let testName of runnableTests) {
+        it(testName, function * () {
+          const selector = makeSelector(makeKey(successKey, testName))
+          yield this.app.client.waitForVisible(selector).then(() => {
+            // got the element for this particular test, check the pass/fail and report from there
+            return this.app.client.getText(`${selector}>.passFail`).then((value) => {
+              if (Array.isArray(value)) {
+                // it's possible to have multiple tests with the same key, mainly due to the
+                // selector character clobbering, these show up as an array of elements
+                throw new Error(`Multiple tests with same selector "${selector}", please modify their names`)
+              }
+              if (value !== 'PASS') {
+                return this.app.client.getText(`${selector}>.failure`).then((value) => {
+                  // attempt to get at least some of the error message from the .failure element to report back
+                  throw new Error(value)
+                })
+              }
+            })
+          })
+        })
+      }
+    }
+
+    const testGroups = Object.keys(tests).filter((k) => typeof tests[k] === 'object')
+    for (let groupKey of testGroups) {
+      // groups of tests, nested objects presumably containing functions
+      describe(groupKey, () => {
+        executeTests(groupKey, makeKey(successKey, groupKey), tests[groupKey])
+      })
+    }
+  }
+
+  executeTests('muon', 'muon', testCases)
 })

test/muon-native/index.js

@@ -0,0 +1,89 @@
+// test cases in this directory, should be resolvable with `require('./X')`
+const testCases = [
+  'urlParseTest',
+  'urlutilTest',
+  'suggestionTest',
+  'siteSuggestionTest'
+]
+
+// load all of the tests into a single large nested object
+const collect = () => {
+  const testList = {}
+  for (let testCase of testCases) {
+    testList[testCase] = require(`./${testCase}`)
+  }
+  return testList
+}
+
+const run = (testRunner) => {
+  // first collect an array of ordered runnable tests, each is async-ish in that it has a
+  // callback but the underlying test may not, we just treat them all that way
+  const runQueue = []
+
+  const buildRunQueue = (successKey, tests) => {
+    const ctx = {} // for `this` within a current nested block
+
+    const makeTestExecutor = (testName, key) => {
+      const beforeAfter = testName === 'before' || testName === 'after'
+      const testFn = tests[testName]
+
+      runQueue.push((cb) => {
+        const thisRunner = testRunner(key)
+        // before and after are async if they have 1 arg, a callback
+        // all other tests take a `test` argument first and if they have a second argument it's a callback
+        if (testFn.length > (beforeAfter ? 0 : 1)) {
+          const _cb = (err) => {
+            thisRunner.ok(!err)
+            thisRunner.done()
+            cb()
+          }
+          testFn.call(ctx, beforeAfter ? _cb : thisRunner, beforeAfter ? undefined : _cb)
+        } else {
+          testFn.call(ctx, beforeAfter ? undefined : thisRunner)
+          thisRunner.done()
+          cb()
+        }
+      })
+    }
+
+    // before() always goes first within a block
+    if (typeof tests.before === 'function') {
+      makeTestExecutor('before', `${successKey} → before`)
+    }
+
+    for (let name of Object.keys(tests)) {
+      if (name === 'before' || name === 'after') {
+        continue
+      }
+      let key = `${successKey} → ${name}`
+      if (typeof tests[name] === 'object') {
+        buildRunQueue(key, tests[name]) // recursive for a nested block of tests
+      } else {
+        makeTestExecutor(name, key)
+      }
+    }
+
+    // after() always goes last within a block
+    if (typeof tests.after === 'function') {
+      makeTestExecutor('after', `${successKey} → after`)
+    }
+  }
+
+  buildRunQueue('muon', collect())
+
+  // now we have a queue of ordered tests, execute in order, async-ish
+  const executeQueue = () => {
+    if (!runQueue.length) {
+      return
+    }
+    const fn = runQueue.shift()
+    fn(() => {
+      setImmediate(executeQueue)
+    })
+  }
+
+  executeQueue()
+}
+
+module.exports.collect = collect
+module.exports.run = run

test/muon-native/siteSuggestionTest.js

@@ -0,0 +1 @@
+module.exports = require('../unit/app/common/lib/siteSuggestionsTestComponents')

test/muon-native/suggestionTest.js

@@ -0,0 +1,12 @@
+// lazy load requires for dual use in and outside muon
+const suggestion = () => require('../../app/common/lib/suggestion')
+
+module.exports = {
+  suggestionSimpleCheck: (test) => {
+    test.deepEqual(suggestion().isSimpleDomainNameValue('http://test.com') &&
+      suggestion().isSimpleDomainNameValue('http://test.com/') &&
+      suggestion().isSimpleDomainNameValue('http://test.com#') &&
+      !suggestion().isSimpleDomainNameValue('http://test.com/test'),
+    true)
+  }
+}

test/muon-native/urlParseTest.js

@@ -0,0 +1,50 @@
+// lazy load requires for dual use in and outside muon
+const urlParse = () => require('../../app/common/urlParse')
+
+const defaultParsedUrl = {
+  hash: '',
+  host: '',
+  hostname: '',
+  href: '',
+  origin: '',
+  path: '/',
+  pathname: '/',
+  port: '',
+  protocol: 'http:',
+  query: '',
+  search: ''
+}
+
+module.exports = {
+  urlParseSimple: (test) => {
+    test.deepEqual(urlParse()('http://bing.com'), Object.assign(defaultParsedUrl, {
+      host: 'bing.com',
+      hostname: 'bing.com',
+      origin: 'http://bing.com/',
+      href: 'http://bing.com/'
+    }))
+  },
+  urlParseComplex: (test) => {
+    test.deepEqual(urlParse()('https://brave.com:333/test?abc=123&def#fff'), {
+      host: 'brave.com:333',
+      hostname: 'brave.com',
+      origin: 'https://brave.com:333/',
+      protocol: 'https:',
+      port: '333',
+      hash: '#fff',
+      pathname: '/test',
+      path: '/test?abc=123&def',
+      search: '?abc=123&def',
+      query: 'abc=123&def',
+      href: 'https://brave.com:333/test?abc=123&def#fff'
+    })
+  },
+  urlParseIssue10270: (test) => {
+    test.deepEqual(urlParse()('http://brave.com%60x.code-fu.org/'), Object.assign(defaultParsedUrl, {
+      host: 'brave.com%60x.code-fu.org',
+      hostname: 'brave.com%60x.code-fu.org',
+      href: 'http://brave.com%60x.code-fu.org/',
+      origin: 'http://brave.com%60x.code-fu.org/'
+    }))
+  }
+}

test/muon-native/urlutilTest.js

@@ -0,0 +1 @@
+module.exports = require('../unit/lib/urlutilTestComponents')