refactor variables store to not be a singleton

Author: corevo

packages/selenium-ide/src/neo/IO/SideeX/ext-command.js

@@ -18,8 +18,7 @@
 import browser from 'webextension-polyfill'
 import Debugger, { convertLocator } from '../debugger'
 import PlaybackState from '../../stores/view/PlaybackState'
-import variables from '../../stores/view/Variables'
-import { Logger, Channels } from '../../stores/view/Logs'
+import { Logger, Channels, output } from '../../stores/view/Logs'
 import FrameNotFoundError from '../../../errors/frame-not-found'
 import { absolutifyUrl } from '../playback/utils'
 import { userAgent as parsedUA } from '../../../common/utils'
@@ -77,12 +76,31 @@ export default class ExtCommand {
         this.setNewTab(details.tabId)
       }
     }
+
+    this.commandVariablesHandler = (message, _sender, sendResponse) => {
+      if (message.getVar) {
+        return sendResponse(this.variables.get(message.variable))
+      } else if (message.storeVar) {
+        this.variables.set(message.storeVar, message.storeStr)
+        return sendResponse(true)
+      } else if (
+        message.log &&
+        output.logs[output.logs.length - 1].message.indexOf(
+          message.log.message
+        ) === -1
+      ) {
+        // this check may be dangerous, especially if something else is bombarding the logs
+        this.logger[message.log.type || 'log'](message.log.message)
+        return sendResponse(true)
+      }
+    }
   }
 
-  async init(baseUrl, testCaseId, options = {}) {
+  async init(baseUrl, testCaseId, options = {}, variables) {
     this.baseUrl = baseUrl
     this.testCaseId = testCaseId
     this.options = options
+    this.variables = variables
     this.waitForNewWindow = false
     this.windowName = ''
     this.windowTimeout = 2000
@@ -116,6 +134,7 @@ export default class ExtCommand {
     browser.webNavigation.onCreatedNavigationTarget.addListener(
       this.newTabHandler
     )
+    browser.runtime.onMessage.addListener(this.commandVariablesHandler)
   }
 
   detach() {
@@ -128,6 +147,7 @@ export default class ExtCommand {
     browser.webNavigation.onCreatedNavigationTarget.removeListener(
       this.newTabHandler
     )
+    browser.runtime.onMessage.removeListener(this.commandVariablesHandler)
   }
 
   getCurrentPlayingWindowSessionIdentifier() {
@@ -360,7 +380,7 @@ export default class ExtCommand {
   async setNewTab(tabId) {
     if (this.waitForNewWindow) {
       this.waitForNewWindow = false
-      variables.set(this.windowName, tabId)
+      this.variables.set(this.windowName, tabId)
     }
     this.windowSession.openedTabIds[
       this.getCurrentPlayingWindowSessionIdentifier()
@@ -631,12 +651,12 @@ export default class ExtCommand {
   }
 
   doStore(string, varName) {
-    variables.set(varName, string)
+    this.variables.set(varName, string)
     return Promise.resolve()
   }
 
   doStoreWindowHandle(varName) {
-    variables.set(varName, this.getCurrentPlayingTabId())
+    this.variables.set(varName, this.getCurrentPlayingTabId())
     return Promise.resolve()
   }
 

packages/selenium-ide/src/neo/IO/SideeX/find-select.js

@@ -40,7 +40,7 @@ async function getActiveTabForTest() {
 
 export async function find(target) {
   try {
-    const xlatedTarget = xlateArgument(target)
+    const xlatedTarget = xlateArgument(target, PlaybackState.variables)
     const tab = await getActiveTabForTest()
     const region = new Region(xlatedTarget)
     await browser.windows.update(tab.windowId, {

packages/selenium-ide/src/neo/IO/SideeX/formatCommand.js

@@ -15,14 +15,9 @@
 // specific language governing permissions and limitations
 // under the License.
 
-import browser from 'webextension-polyfill'
-import { Logger, Channels, output } from '../../stores/view/Logs'
-import variables from '../../stores/view/Variables'
-
-const logger = new Logger(Channels.PLAYBACK)
 const nbsp = String.fromCharCode(160)
 
-export function xlateArgument(value) {
+export function xlateArgument(value, variables) {
   value = value.replace(/^\s+/, '')
   value = value.replace(/\s+$/, '')
   let r2
@@ -56,7 +51,7 @@ export function xlateArgument(value) {
   }
 }
 
-export function interpolateScript(script) {
+export function interpolateScript(script, variables) {
   let value = script.replace(/^\s+/, '').replace(/\s+$/, '')
   let r2
   let parts = []
@@ -106,24 +101,3 @@ function string(value) {
     return ''
   }
 }
-
-function handleFormatCommand(message, _sender, sendResponse) {
-  if (message.getVar) {
-    return sendResponse(variables.get(message.variable))
-  } else if (message.storeVar) {
-    variables.set(message.storeVar, message.storeStr)
-    return sendResponse(true)
-  } else if (
-    message.log &&
-    output.logs[output.logs.length - 1].message.indexOf(message.log.message) ===
-      -1
-  ) {
-    // this check may be dangerous, especially if something else is bombarding the logs
-    logger[message.log.type || 'log'](message.log.message)
-    return sendResponse(true)
-  }
-}
-
-try {
-  browser.runtime.onMessage.addListener(handleFormatCommand)
-} catch (e) {} // eslint-disable-line

packages/selenium-ide/src/neo/IO/SideeX/playback.js

@@ -29,13 +29,13 @@ let ignoreBreakpoint = false
 let breakOnNextCommand = false
 let executor = undefined
 
-export function play(currUrl, exec) {
+export function play(currUrl, exec, variables) {
   baseUrl = currUrl
   ignoreBreakpoint = false
   breakOnNextCommand = false
   executor = exec
   initPlaybackTree()
-  return prepareToPlay()
+  return prepareToPlay(variables)
     .then(executionLoop)
     .then(finishPlaying)
     .catch(catchPlayingError)
@@ -180,12 +180,17 @@ function runNextCommand() {
   }
 }
 
-function prepareToPlay() {
-  return executor.init(baseUrl, PlaybackState.currentRunningTest.id, {
-    // softInit will try to reconnect to the last session for the sake of running the command if possible
-    softInit:
-      PlaybackState.isSingleCommandRunning || PlaybackState.isPlayFromHere,
-  })
+function prepareToPlay(variables) {
+  return executor.init(
+    baseUrl,
+    PlaybackState.currentRunningTest.id,
+    {
+      // softInit will try to reconnect to the last session for the sake of running the command if possible
+      softInit:
+        PlaybackState.isSingleCommandRunning || PlaybackState.isPlayFromHere,
+    },
+    variables
+  )
 }
 
 function prepareToPlayAfterConnectionFailed() {

packages/selenium-ide/src/neo/IO/playback/webdriver.js

@@ -17,7 +17,6 @@
 
 import webdriver from 'browser-webdriver'
 import { absolutifyUrl } from './utils'
-import variables from '../../stores/view/Variables'
 import { Logger, Channels } from '../../stores/view/Logs'
 import PlaybackState from '../../stores/view/PlaybackState'
 
@@ -38,8 +37,9 @@ export default class WebDriverExecutor {
     this.logger = new Logger(Channels.PLAYBACK)
   }
 
-  async init(baseUrl) {
+  async init({ baseUrl, variables }) {
     this.baseUrl = baseUrl
+    this.variables = variables
     this.driver = await new webdriver.Builder()
       .withCapabilities(this.capabilities)
       .usingServer(this.server)
@@ -219,7 +219,7 @@ export default class WebDriverExecutor {
       ...script.argv
     )
     if (optionalVariable) {
-      variables.set(optionalVariable, result)
+      this.variables.set(optionalVariable, result)
     }
   }
 
@@ -231,33 +231,33 @@ export default class WebDriverExecutor {
       ...script.argv
     )
     if (optionalVariable) {
-      variables.set(optionalVariable, result)
+      this.variables.set(optionalVariable, result)
     }
   }
 
   // store commands
 
   async doStore(string, variable) {
-    variables.set(variable, string)
+    this.variables.set(variable, string)
     return Promise.resolve()
   }
 
   async doStoreText(locator, variable) {
     const element = await waitForElement(locator, this.driver)
     const text = await element.getText()
-    variables.set(variable, text)
+    this.variables.set(variable, text)
   }
 
   async doStoreValue(locator, variable) {
     const element = await waitForElement(locator, this.driver)
     const value = await element.getAttribute('value')
-    variables.set(variable, value)
+    this.variables.set(variable, value)
   }
 
   // assertions
 
   async doAssert(variableName, value) {
-    const variable = `${variables.get(variableName)}`
+    const variable = `${this.variables.get(variableName)}`
     if (variable != value) {
       throw new Error(
         "Actual value '" + variable + "' did not match '" + value + "'"

packages/selenium-ide/src/neo/__test__/IO/playback/formatCommand.spec.js

@@ -19,44 +19,45 @@ import {
   interpolateScript,
   xlateArgument,
 } from '../../../IO/SideeX/formatCommand'
-import variables from '../../../stores/view/Variables'
+import Variables from '../../../stores/view/Variables'
 
-afterEach(() => {
-  variables.clear()
+let variables
+beforeEach(() => {
+  variables = new Variables()
 })
 
 describe('interpolate string', () => {
   it('should interpolate false values', () => {
     variables.set('a', undefined)
-    expect(xlateArgument('${a}')).toBe('undefined')
+    expect(xlateArgument('${a}', variables)).toBe('undefined')
     variables.set('a', null)
-    expect(xlateArgument('${a}')).toBe('null')
+    expect(xlateArgument('${a}', variables)).toBe('null')
     variables.set('a', false)
-    expect(xlateArgument('${a}')).toBe('false')
+    expect(xlateArgument('${a}', variables)).toBe('false')
     variables.set('a', 0)
-    expect(xlateArgument('${a}')).toBe('0')
+    expect(xlateArgument('${a}', variables)).toBe('0')
     variables.set('a', '')
-    expect(xlateArgument('${a}')).toBe('')
+    expect(xlateArgument('${a}', variables)).toBe('')
   })
 })
 
 describe('interpolate script', () => {
   it('should not interpolate a script without variables', () => {
     const script = 'return 1'
-    expect(interpolateScript(script).script).toEqual(script)
+    expect(interpolateScript(script, variables).script).toEqual(script)
   })
   it('should interpolate a script with a single argument', () => {
     variables.set('a', 1)
     const script = 'return ${a}'
-    const r = interpolateScript(script)
+    const r = interpolateScript(script, variables)
     expect(r.script).toEqual('return arguments[0]')
     expect(r.argv[0]).toBe(1)
   })
   it('should interpolate a script with multiple arguments', () => {
     variables.set('a', 1)
     variables.set('b', false)
     const script = 'return ${a} + ${a} || ${b}'
-    const r = interpolateScript(script)
+    const r = interpolateScript(script, variables)
     expect(r.script).toEqual(
       'return arguments[0] + arguments[0] || arguments[1]'
     )

packages/selenium-ide/src/neo/__test__/playback/playback-tree/command-node.spec.js

@@ -17,6 +17,7 @@
 
 import Command, { ControlFlowCommandNames } from '../../../models/Command'
 import { CommandNode } from '../../../playback/playback-tree/command-node'
+import Variables from '../../../stores/view/Variables'
 
 describe('Command Node', () => {
   it('control flow check returns correct result', () => {
@@ -69,7 +70,7 @@ describe('Command Node', () => {
       ''
     )
     const node = new CommandNode(command)
-    node._evaluate().then(result => {
+    node._evaluate({ variables: new Variables() }).then(result => {
       expect(result.result).toEqual('Invalid number provided as a target.')
     })
   })

packages/selenium-ide/src/neo/__test__/stores/view/Variables.spec.js

@@ -16,52 +16,52 @@
 // under the License.
 
 import { useStrict } from 'mobx'
-import Variables from '../../../stores/view/Variables'
+import Variables from '../../../stores/view/variables'
 
 useStrict(true)
 
-afterEach(() => {
-  Variables.clear()
-})
-
-describe('Variables', () => {
+describe('variables', () => {
+  let variables
+  beforeEach(() => {
+    variables = new Variables()
+  })
   it('should clear the map', () => {
     const key = 'key1'
     const value = 'value1'
-    Variables.set(key, value)
-    expect(Variables.storedVars.size).toBe(1)
-    Variables.clear()
-    expect(Variables.storedVars.size).toBe(0)
+    variables.set(key, value)
+    expect(variables.storedVars.size).toBe(1)
+    variables.clear()
+    expect(variables.storedVars.size).toBe(0)
   })
   it('should add new key and value to the map', () => {
     const key = 'key1'
     const value = 'value1'
-    Variables.set(key, value)
-    expect(Variables.storedVars.size).toBe(1)
+    variables.set(key, value)
+    expect(variables.storedVars.size).toBe(1)
   })
   it('should not add duplicates to the map', () => {
     const key = 'key1'
     const value = 'value1'
     const value2 = 'value2'
-    Variables.set(key, value)
-    Variables.set(key, value2)
-    expect(Variables.storedVars.size).toBe(1)
+    variables.set(key, value)
+    variables.set(key, value2)
+    expect(variables.storedVars.size).toBe(1)
   })
   it('should get the value correctly from the map', () => {
     const key1 = 'key1'
     const value1 = 'value1'
     const key2 = 'key2'
     const value2 = 'value2'
-    Variables.set(key1, value1)
-    Variables.set(key2, value2)
-    expect(Variables.get(key1)).toBe(value1)
+    variables.set(key1, value1)
+    variables.set(key2, value2)
+    expect(variables.get(key1)).toBe(value1)
   })
   it('should delete the key and the value from the map', () => {
     const key = 'key1'
     const value = 'value1'
-    Variables.set(key, value)
-    expect(Variables.storedVars.size).toBe(1)
-    Variables.delete(key)
-    expect(Variables.storedVars.size).toBe(0)
+    variables.set(key, value)
+    expect(variables.storedVars.size).toBe(1)
+    variables.delete(key)
+    expect(variables.storedVars.size).toBe(0)
   })
 })