diff --git a/migrations/20160121091841-create-comment.js b/migrations/20160121091841-create-comment.js deleted file mode 100644 index 6aacd2d..0000000 --- a/migrations/20160121091841-create-comment.js +++ /dev/null @@ -1,39 +0,0 @@ -'use strict'; -module.exports = { - up: function(queryInterface, Sequelize) { - return queryInterface.createTable('Comments', { - id: { - allowNull: false, - autoIncrement: true, - primaryKey: true, - type: Sequelize.INTEGER - }, - repo: { - type: Sequelize.TEXT - }, - pr: { - type: Sequelize.INTEGER - }, - filename: { - type: Sequelize.TEXT - }, - line: { - type: Sequelize.INTEGER - }, - feature: { - type: Sequelize.TEXT - }, - createdAt: { - allowNull: false, - type: Sequelize.DATE - }, - updatedAt: { - allowNull: false, - type: Sequelize.DATE - } - }); - }, - down: function(queryInterface, Sequelize) { - return queryInterface.dropTable('Comments'); - } -}; diff --git a/models/comment.js b/models/comment.js deleted file mode 100644 index 726f0b9..0000000 --- a/models/comment.js +++ /dev/null @@ -1,17 +0,0 @@ -'use strict'; -module.exports = function(sequelize, DataTypes) { - var Comment = sequelize.define('Comment', { - repo: DataTypes.TEXT, - pr: DataTypes.INTEGER, - filename: DataTypes.TEXT, - line: DataTypes.INTEGER, - feature: DataTypes.TEXT - }, { - classMethods: { - associate: function(models) { - // associations can be defined here - } - } - }); - return Comment; -}; diff --git a/routes/hook/index.js b/routes/hook/index.js index 965f6e2..a704c50 100644 --- a/routes/hook/index.js +++ b/routes/hook/index.js @@ -70,51 +70,36 @@ function processPullRequest(destinationRepo, originRepo, originBranch, prNumber, processor.process(githubClient, originRepo, originBranch, file, discordConfig, function(incompatibility) { // Callback for handling an incompatible line of code var line = diff.lineToIndex(file.patch, incompatibility.usage.source.start.line); - var filename = file.filename; - var feature = incompatibility.featureData.title; - var comment = feature + ' not supported by: ' + incompatibility.featureData.missing; - - var alreadyCommented = commentExists(destinationRepo, prNumber, filename, line, feature); - alreadyCommented.then(function(alreadyCommented) { - if (!alreadyCommented) { - var redisQueue = kue.createQueue({ - redis: config.redisURL - }); - - // Create a Redis job that will submit the comment - var commentJob = redisQueue.create('comment', { - commentURL: commentURL, - sha: currentCommit.sha, - filename: filename, - line: line, - comment: comment - }); - - // If the comment is rejected, re-attempt several times with - // exponentially longer waits between each attempt. - commentJob.attempts(config.commentAttempts); - commentJob.backoff({ - type: 'exponential' - }); - - // If the comment is rejected after several attempts, log an - // error. - commentJob.on('failed', function() { - logger.error('Error posting comment to line ' + line + ' of ' + filename + ' in ' + originRepo + ' pull request #' + prNumber); - }); - - commentJob.save(function(error) { - if (error) return logger.error(error); - }); - - models.Comment.create({ - repo: destinationRepo, - pr: prNumber, - filename: filename, - line: line, - feature: feature - }); - } + var comment = incompatibility.featureData.title + ' not supported by: ' + incompatibility.featureData.missing; + + var redisQueue = kue.createQueue({ + redis: config.redisURL + }); + + // Create a Redis job that will submit the comment + var commentJob = redisQueue.create('comment', { + commentURL: commentURL, + sha: currentCommit.sha, + filename: file.filename, + line: line, + comment: comment + }); + + // If the comment is rejected, re-attempt several times with + // exponentially longer waits between each attempt. + commentJob.attempts(config.commentAttempts); + commentJob.backoff({ + type: 'exponential' + }); + + // If the comment is rejected after several attempts, log an + // error. + commentJob.on('failed', function() { + logger.error('Error posting comment to line ' + line + ' of ' + file.filename + ' in ' + originRepo + ' pull request #' + prNumber); + }); + + commentJob.save(function(error) { + if (error) return logger.error(error); }); }); @@ -192,34 +177,4 @@ function getCommitDetail(repo, sha) { return deferred.promise; } -function commentExists(repo, prNumber, filename, line, feature) { - var deferred = Q.defer(); - - models.Comment.count({ - where: { - repo: repo, - pr: prNumber, - filename: filename, - line: line, - feature: feature - } - }).then( - // Success callback (a matching comment was found) - function(count) { - if (count > 0) { - deferred.resolve(true); - } else { - deferred.resolve(false); - } - }, - - // Failure callback (no matching comments were found) - function(error) { - deferred.reject(error); - } - ); - - return deferred.promise; -} - module.exports = router; diff --git a/tests/test-utils.js b/tests/test-utils.js index 3cceec3..5ee3a89 100644 --- a/tests/test-utils.js +++ b/tests/test-utils.js @@ -3,8 +3,6 @@ var fs = require('fs'); var path = require('path'); -var nock = require('nock'); - var config = require('../lib/config'); module.exports = { @@ -34,40 +32,4 @@ module.exports = { return JSON.parse(fs.readFileSync(directory + '/' + fixture + '.json')); }, - // Utility to setup the nock and lock variables into place - setupNock: function(url, item, requestType, payload, posts, done) { - var completedPosts = 0; - - nock(this.githubHost).persist()[requestType](url).reply(function() { - if (requestType === 'post') completedPosts++; - - if (completedPosts === posts) { - done(); - nock.cleanAll(); // Cleanup so there's no interfering with other tests - } - - return [200, payload]; - }); - }, - - // Utility to setup all nocks for a manifest - setupNocksForManifest: function(manifest, index, done) { - var item; - for (var url in manifest.urls) { - if (manifest.urls.hasOwnProperty(url)) { - - item = manifest.urls[url]; - - this.setupNock( - url, - item, - item.method.toLowerCase(), - this.getFileContents(this.recordedFixturesDir + index.toString(), item.file), - manifest.posts, - done - ); - } - } - - } }; diff --git a/tests/tests.js b/tests/tests.js index a7e7c6f..2de8d91 100644 --- a/tests/tests.js +++ b/tests/tests.js @@ -7,14 +7,14 @@ var fs = require('fs'); var chai = require('chai'); var request = require('request'); +var nock = require('nock'); var assert = chai.assert; +var testUtils = require('./test-utils'); var config = require('../lib/config'); var www = require('../bin/www'); var models = require('../models'); -var testUtils = require('./test-utils'); - var notFoundURL = testUtils.appHost + '/page-that-will-never-exist'; describe('Discord Tests', function() { @@ -93,10 +93,41 @@ describe('Discord Tests', function() { // Start the test definition it('Recorded test ' + plainIndex + ': ' + manifest.description, function(done) { - testUtils.setupNocksForManifest(manifest, plainIndex, done); + var item; + + for (var url in manifest.urls) { + if (manifest.urls.hasOwnProperty(url)) { + + item = manifest.urls[url]; + + setupNock( + url, + item, + item.method.toLowerCase(), + testUtils.getFileContents(testUtils.recordedFixturesDir + plainIndex.toString(), item.file), + manifest + ); + } + } // Kick the test off sendHookPayload(testUtils.recordedFixturesDir + plainIndex.toString()); + + // Utility to setup the nock and lock variables into place + function setupNock(url, item, requestType, payload, manifest) { + var completedPosts = 0; + + nock(testUtils.githubHost).persist()[requestType](url).reply(function() { + if (requestType === 'post') completedPosts++; + + if (completedPosts === manifest.posts) { + done(); + nock.cleanAll(); // Cleanup so there's no interfering with other tests + } + + return [200, payload]; + }); + } }); }); @@ -119,7 +150,6 @@ describe('Discord Tests', function() { */ describe('Database Tests', function() { describe('Ping', function() { - it('Ping events are recorded', function(done) { models.Ping.count().then(function(countBeforePing) { assert.equal(countBeforePing, 0); @@ -165,13 +195,11 @@ describe('Discord Tests', function() { }); }); }); - - }); // End Ping tests - }); // End Database tests + }); + }); /** * We need to do cleanup so that the tests don't hang - * TODO: move test cleanup to a "tearDown" method? */ describe('Test Cleanup', function() { it('Server closes properly', function() {