groovecoder / discord

GitHub webhook that analyzes pull requests and adds comments about incompatible CSS
Mozilla Public License 2.0
29 stars 13 forks source link

Adding recorder and restructuring test framework #132

Closed darkwing closed 9 years ago

darkwing commented 9 years ago

Usage at the top of recorder.js

openjck commented 9 years ago

Discord recorder or...

Recorder Discord

openjck commented 9 years ago

This is unrelated to these changes, so we don't need to block on it. But the changes got me thinking...

At the moment, we manually send an HTTP request to post the comment. At some point in the future, octonode should implement create_pull_request_comment. At that point, we'll have the option of making our code a little cleaner by getting rid of commenter.js and using their implementation instead. If we decide to do that, would we have other ways of testing that comments are submitted? It's not something we need to worry about right now, but maybe we should keep the question in mind for the future.

darkwing commented 9 years ago

Again, it doesn't matter who what when where why how the comment is posted. If it's done via HTTP, nock gets it. No changes.

openjck commented 9 years ago

Again, it doesn't matter who what when where why how the comment is posted. If it's done via HTTP, nock gets it. No changes.

Oh awesome! Okay cool. Thanks for letting me know. Happy to hear it won't be a problem. :-)

openjck commented 9 years ago

I'm seeing this error:

$ node recorder --test=1 --description="This test describes xyz conditions"
url.js:110
    throw new TypeError("Parameter 'url' must be a string, not " + typeof url)
          ^
TypeError: Parameter 'url' must be a string, not undefined
    at Url.parse (url.js:110:11)
    at Object.urlParse [as parse] (url.js:104:5)
    at Object.<anonymous> (/Users/john-moz/devel/forks/discord/discord/redisQueue.js:19:26)

The error goes away if the following line is added to recorder.js:

process.env.REDIS_URL = 'redis://localhost';
darkwing commented 9 years ago

Locally? I don't need that at all

darkwing commented 9 years ago

I think this is ready to go again

openjck commented 9 years ago

Looks good! We could keep the coding style consistent by running gulp beautify.

darkwing commented 9 years ago

Done

openjck commented 9 years ago

Awesome stuff!