New Script Pattern

[michaelansel]

I had a crazy pipe dream idea for a new script format that I think will still be backwards-compatible, be fairly powerful, and not require programmatic launching (#858). There is a lot in this example, so I've added lots of comments and will go through each piece. In short, it probably works as you would expect it to.

# this replaces module.exports = (robot) ->
class Sarcasm extends HubotScript
  constructor: (@robot) ->
    super @robot, 'sarcasm' # give every script a unique name

  # Specify config requirements
  config:
    'HUBOT_SARCASM_CLEAN':
      required: true # require that this config option is set
      description: "Keep everything safe for work" # used to generate help/error messages
      validation: (value) -> # maybe a function to parse/validate the input?
    'HUBOT_SARCASM_LEVEL':
      required: false # optional (feels a little redundant with default)
      default: 7 # default to 7
      description: "How sarcastic should hubot be? (1-7)"

  # Specify dependencies
  dependencies:
    capabilities:
      'private-message' # require an adapter that supports sending direct/private messages
    middleware:
      'ldap-user-info' # need this middleware to be loaded

  # Data for a theoretical extension that runs automated integration tests against all loaded scripts
  examples:
    # say something and then expect to the pattern to be matched
    "I'm just saying hello...": /sup/
    "Hubot: orly": /^User: rly rly$/

  # Specify all routes/listeners
  routes:
    # equivalent to:  robot.hear /hello/, (msg) -> msg.say 'sup'
    'hello':
      command: true # injects the "respond" prefix
      pattern: /hello/
      handler: (response) ->
        response.say 'sup'

    'orly':
      command: false
      help:
        'Hubot: orly': "Troll anyone who says 'orly' to Hubot"
      pattern: /orly/
      handler: (response, done) -> # arity == 2 causes this to be treated as an async function
        @doSomeAwesome response, done

    'say-hi-to-new-people':
      event: 'enter'
      handler: (response) ->
        response.send 'hi new person!'

    'sass-at-new-topics':
      event: 'topic'
      handler: (response) ->
        response.reply 'orly?'

  # Helper functions (that are now easily accessible for unit testing!)
  doSomeAwesome: (response, cb) ->
    process.nextTick ->
      response.reply 'rly rly'
      cb()

module.exports = Sarcasm

First, we now use a class instead of just a function. This makes it easy and obvious for adding helper methods that are accessible by unit tests. This would require a one-line change to loadFile, but I'm pretty sure this would be backwards compatible with existing scripts.

Second, metadata about the script is specified as properties. This allows easy access at any time to any of the properties of a script and could enable magic extensions (like automated integration tests).

Routes are enumerated so you can directly access the handler for any given listener for unit testing. Help information is now associated with the listener and is accessible without needing to parse comments.

The HubotScript parent class would take care of doing all "new script" actions (like registering routes and validating config/dependencies), so we just need to have loadFile use new instead of calling the result of require directly.

Moving to a more data-driven script model might also make hot reloading easier (#715) because we could have a clear list of everything the script has added to the global Robot context (would need to block monkey patches).

I'm still stewing on this (particularly the idea of async route definition), but I think this is a pretty clean foundation that will require a one line code change (and approximately a million lines of documentation changes plus updates to the generator).

[geoffreyanderson]

I like the idea and the doors it opens for testability. Also enjoy the idea of having config information declared very explicitly in here. Looking at it some more though, it feels "too complex" for a simple script that you may want to have for easy interactions with some REST API.

Given the examples in your test snippet, what's explicitly required in the class to make sure a script is structured properly and can be loaded? Is there some sort of error checking to be done on each script class that gets loaded in?

[technicalpickles]

cc #249 for a slightly different take

[technicalpickles]

We're going to use Hubot Evolution
for new changes like this. @bkeepers started one for something in this area over at hubotio/evolution#2