From f5a6070a12ae0c0b2088036f70735103632b6618 Mon Sep 17 00:00:00 2001 From: Spain Train Date: Wed, 2 Oct 2013 18:00:18 -0400 Subject: [PATCH 1/5] #25 Add tests for bug --- package.json | 4 ++- test/server.test.js | 85 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 1 deletion(-) create mode 100644 test/server.test.js diff --git a/package.json b/package.json index 864d558..c23dd4e 100644 --- a/package.json +++ b/package.json @@ -30,7 +30,9 @@ "mocha": "latest", "chai": "latest", "debug": "~0.7.2", - "ini": "~1.1.0" + "ini": "~1.1.0", + "proxyquire": "~0.5.1", + "sinon": "~1.7.3" }, "optionalDependencies": {}, "engines": { diff --git a/test/server.test.js b/test/server.test.js new file mode 100644 index 0000000..f3ca616 --- /dev/null +++ b/test/server.test.js @@ -0,0 +1,85 @@ +'use strict'; + +var proxyquire = require('proxyquire'), + expect = require('chai').expect, + sinon = require('sinon'); + +var VERBS = ['get', 'post', 'put', 'del']; + +describe('reggie npm server (unit)', function () { + var reggieServer, + req = {}, + res = {}, + routes = {}, + restify = {}, + data = function Data() {}; + + var versions = [ + '1.0.0', + '1.0.1', + '1.0.2-2', + '1.0.2-10' + ] + data.prototype.packageMeta = function () { + return { + versions: versions.reduce(function (verObj, currVer) { + verObj[currVer] = {data: {}, time: {}}; + return verObj; + }, {}) + }; + }; + data.prototype.whichVersions = function () { + return versions; + }; + + beforeEach(function () { + //Store route functions in a hash + VERBS.forEach(function (verb) { routes[verb] = {}; }); + + //mock restify + restify.createServer = function() { + var serverMock = { + 'use': function () {}, + 'listen': function () {} + }; + VERBS.forEach(function (verb) { + serverMock[verb] = function (route, cb) { + routes[verb][route] = cb; + }; + }); + return serverMock; + }; + + data.prototype.init = function () {}; + + reggieServer = proxyquire('../server', { + 'restify': restify, + './lib/data': data + }); + + req.params = { + name: 'test' + }; + + res.json = sinon.spy(); + }); + + describe('/:name', function () { + it('Should choose "latest" by semver order', function () { + routes.get['/:name'](req, res); + expect(res.json.firstCall.args[1]['dist-tags'].latest).to.equal('1.0.2-10'); + }); + }); + + describe('/-/all', function () { + it('Should choose "latest" by semver order', function () { + data.prototype.getAllPackageNames = function () { + return ['test']; + }; + routes.get['/-/all'](req, res); + console.log(res.json.firstCall.args[1]); + expect(res.json.firstCall.args[1].test['dist-tags'].latest).to.equal('1.0.2-10'); + expect(res.json).to.haveBeenCalled; + }); + }); +}); \ No newline at end of file From 9c0dd6f2095a8d612fbcd46b630ef9c5056eaa97 Mon Sep 17 00:00:00 2001 From: Spain Train Date: Wed, 2 Oct 2013 18:36:04 -0400 Subject: [PATCH 2/5] #25 Add tests for sort order --- test/server.test.js | 40 ++++++++++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/test/server.test.js b/test/server.test.js index f3ca616..e6b55ad 100644 --- a/test/server.test.js +++ b/test/server.test.js @@ -2,7 +2,8 @@ var proxyquire = require('proxyquire'), expect = require('chai').expect, - sinon = require('sinon'); + sinon = require('sinon'), + semver = require('semver'); var VERBS = ['get', 'post', 'put', 'del']; @@ -19,7 +20,7 @@ describe('reggie npm server (unit)', function () { '1.0.1', '1.0.2-2', '1.0.2-10' - ] + ]; data.prototype.packageMeta = function () { return { versions: versions.reduce(function (verObj, currVer) { @@ -57,29 +58,48 @@ describe('reggie npm server (unit)', function () { './lib/data': data }); - req.params = { - name: 'test' - }; - + req.params = { name: 'test' }; res.json = sinon.spy(); }); describe('/:name', function () { - it('Should choose "latest" by semver order', function () { + beforeEach(function () { routes.get['/:name'](req, res); + }); + + it('Should choose latest by semver order', function () { expect(res.json.firstCall.args[1]['dist-tags'].latest).to.equal('1.0.2-10'); }); + + it('Should "sort" by semver order', function () { + // Yes, this is depending on an implementation detail of Object.keys + // Yes, that violates the API contract of Object and is bad + // Yes, this is how the npm client actually works + var resultVersions; + resultVersions = Object.keys(res.json.firstCall.args[1].versions); + expect(resultVersions).to.deep.equal(versions.sort(semver.compare)); + }); }); describe('/-/all', function () { - it('Should choose "latest" by semver order', function () { + beforeEach(function () { data.prototype.getAllPackageNames = function () { return ['test']; }; routes.get['/-/all'](req, res); - console.log(res.json.firstCall.args[1]); + }); + + it('Should choose "latest" by semver order', function () { expect(res.json.firstCall.args[1].test['dist-tags'].latest).to.equal('1.0.2-10'); - expect(res.json).to.haveBeenCalled; + }); + + it('Should "sort" by semver order', function () { + // Yes, this is depending on an implementation detail of Object.keys + // Yes, that violates the API contract of Object and is bad + // Yes, this is how the npm client actually works + var resultVersions; + resultVersions = Object.keys(res.json.firstCall.args[1].test.versions); + expect(resultVersions).to.deep.equal(versions.sort(semver.compare)); }); }); }); \ No newline at end of file From 38d960fbb1371cfcbf6107dbea9ee740b0d88610 Mon Sep 17 00:00:00 2001 From: Spain Train Date: Wed, 2 Oct 2013 18:36:36 -0400 Subject: [PATCH 3/5] #25 Fix /:name --- server.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server.js b/server.js index b48f75b..af10e91 100644 --- a/server.js +++ b/server.js @@ -159,7 +159,7 @@ server.get('/:name', function (req, res) { var meta = data.packageMeta(packageName); if (!meta) return notFound(res); - var versions = data.whichVersions(packageName).sort(); + var versions = data.whichVersions(packageName).sort(semver.compare); var versionsData = {}; var times = {}; versions.forEach(function(v) { @@ -173,7 +173,7 @@ server.get('/:name', function (req, res) { name: meta.name, description: meta.description, 'dist-tags': { - latest: versions[versions.length-1] + latest: semver.maxSatisfying(versions, '*') }, versions: versionsData, maintainers: [], From d05bc3a47bb8753b4c096dab9ed302d8bd6742f8 Mon Sep 17 00:00:00 2001 From: Spain Train Date: Wed, 2 Oct 2013 18:40:11 -0400 Subject: [PATCH 4/5] #25 Fix listAction (/-/all and /-/all/since) --- server.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server.js b/server.js index af10e91..1f43c5d 100644 --- a/server.js +++ b/server.js @@ -210,9 +210,9 @@ function listAction(req, res) { res.json(200, result); function getPackageInfo(packageName) { - var versions = data.whichVersions(packageName).sort(); + var versions = data.whichVersions(packageName).sort(semver.compare); var meta = data.packageMeta(packageName); - var lastVersion = versions[versions.length-1]; + var lastVersion = semver.maxSatisfying(versions, '*'); var versionsData = {}; versions.forEach(function(v) { versionsData[v] = 'latest'; From 8a80e719a882c7d86d4645ed9582e3540c49090e Mon Sep 17 00:00:00 2001 From: Spain Train Date: Thu, 3 Oct 2013 14:28:08 -0400 Subject: [PATCH 5/5] #25 DRYed up version related code at maintainer request --- server.js | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/server.js b/server.js index 1f43c5d..1dd1550 100644 --- a/server.js +++ b/server.js @@ -159,10 +159,10 @@ server.get('/:name', function (req, res) { var meta = data.packageMeta(packageName); if (!meta) return notFound(res); - var versions = data.whichVersions(packageName).sort(semver.compare); + var versions = getPackageVersions(packageName); var versionsData = {}; var times = {}; - versions.forEach(function(v) { + versions.all.forEach(function(v) { versionsData[v] = meta.versions[v].data; times[v] = meta.versions[v].time; }); @@ -173,7 +173,7 @@ server.get('/:name', function (req, res) { name: meta.name, description: meta.description, 'dist-tags': { - latest: semver.maxSatisfying(versions, '*') + latest: versions.latest }, versions: versionsData, maintainers: [], @@ -210,11 +210,10 @@ function listAction(req, res) { res.json(200, result); function getPackageInfo(packageName) { - var versions = data.whichVersions(packageName).sort(semver.compare); + var versions = getPackageVersions(packageName); var meta = data.packageMeta(packageName); - var lastVersion = semver.maxSatisfying(versions, '*'); var versionsData = {}; - versions.forEach(function(v) { + versions.all.forEach(function(v) { versionsData[v] = 'latest'; }); @@ -223,19 +222,27 @@ function listAction(req, res) { name: meta.name, description: meta.description, 'dist-tags': { - latest: lastVersion + latest: versions.latest }, versions: versionsData, maintainers: [], author: meta.author, repository: meta.repository, time: { - modified: meta.versions[lastVersion].time + modified: meta.versions[versions.latest].time } }; } } +function getPackageVersions(packageName) { + var versions = data.whichVersions(packageName).sort(semver.compare); + return { + all: versions, + latest: semver.maxSatisfying(versions, '*') + }; +} + server.put('/:name/-/:filename/-rev/:rev', function (req, res) { var filename = req.params.filename; var rand = Math.floor(Math.random()*4294967296).toString(36);