From 0bbaf5ff938804c2b21b9a40ef2c607d81293e96 Mon Sep 17 00:00:00 2001 From: Kieran Sedgwick Date: Wed, 21 May 2014 13:07:31 -0400 Subject: [PATCH] Added and fixed unit tests Aside from testing our new module (network.js), we also had to add logic that would test Filer appropriately in both the node and browser environments. --- .travis.yml | 2 +- CONTRIBUTING.md | 11 +++-- build/node_wrap.start | 7 ++- src/shell/shell.js | 14 +++--- tests/require-config.js | 1 + tests/spec/fs.open.spec.js | 4 +- tests/spec/lib.spec.js | 60 ++++++++++++++++-------- tests/spec/node-js/simple/nodejs.spec.js | 13 +++++ tests/spec/shell/wget.spec.js | 26 ++-------- tests/spec/shell/zip-unzip.spec.js | 41 +++++++--------- tests/test-manifest.js | 3 ++ 11 files changed, 102 insertions(+), 80 deletions(-) create mode 100644 tests/spec/node-js/simple/nodejs.spec.js diff --git a/.travis.yml b/.travis.yml index 789f9fb..6490d4e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,6 +1,6 @@ language: node_js node_js: - - "0.11" + - "0.10" before_install: npm install -g grunt-cli notifications: email: false diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5a3c75c..a4d015f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -21,7 +21,8 @@ You can now run the following grunt tasks: * `grunt check` will run [JSHint](http://www.jshint.com/) on your code (do this before submitting a pull request) to catch errors * `grunt develop` will create a single file version of the library for testing in `dist/idbfs.js` * `grunt release` like `develop` but will also create a minified version of the library in `dist/idbfs.min.js` -* `grunt test` will run [JSHint](http://www.jshint.com/) on your code and the test suite in [PhantomJS](http://phantomjs.org/) +* `grunt test` or `grunt test-node` will run [JSHint](http://www.jshint.com/) on your code and the test suite in the context of `nodejs` +* `grunt test-browser` will run [JSHint](http://www.jshint.com/) and start a localhost server on port `1234`. Navigating to `localhost:1234/tests/index.html` will run the test suite in the context of the browser. **NOTE:** When finished, you will have to manually shut off the server by pressing `cmd/ctrl`+`c` in the same terminal session you ran `grunt test-browser`. Once you've done some hacking and you'd like to have your work merged, you'll need to make a pull request. If you're patch includes code, make sure to check that all the @@ -63,14 +64,14 @@ The user *must* be on their local `develop` branch before running any form of `g ## Tests Tests are writting using [Mocha](http://visionmedia.github.io/mocha/) and [Chai](http://chaijs.com/api/bdd/). -You can run the tests in your browser by opening the `tests` directory. You can also run them -[here](http://js-platform.github.io/filer/tests/). +You can run the tests in your browser by running `grunt test-browser` and opening the `tests` directory @ `http://localhost:1234/tests`. There are a number of configurable options for the test suite, which are set via query string params. -First, you can choose which filer source to use (i.e., src/, dist/filer.js or dist/filer.min.js). -The default is to use what is in /src, and you can switch to built versions like so: +First, you can choose which filer source to use (i.e., src/, dist/filer-test.js, dist/filer.js or dist/filer.min.js). +The default is to use what is in /dist/filer-test.js, and you can switch to other versions like so: * tests/index.html?filer-dist/filer.js * tests/index.html?filer-dist/filer.min.js +* tests/index.html?filer-src/filer.js (from src) Second, you can specify which provider to use for all non-provider specific tests (i.e., most of the tests). The default provider is `Memory`, and you can switch it like so: diff --git a/build/node_wrap.start b/build/node_wrap.start index cb49677..4cddf7d 100644 --- a/build/node_wrap.start +++ b/build/node_wrap.start @@ -12,5 +12,10 @@ THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND */ (function( root, factory ) { - module.exports = factory(); + if (typeof exports === "object") { + module.exports = factory(); + } else if (typeof define === "function" && define.amd) { + // AMD. Register as an anonymous module. + define( factory ); + } }( this, function() { diff --git a/src/shell/shell.js b/src/shell/shell.js index 167b1b0..545ec95 100644 --- a/src/shell/shell.js +++ b/src/shell/shell.js @@ -445,17 +445,19 @@ define(function(require) { return; } - // Grab whatever is after the last / (assuming there is one) and - // remove any non-filename type chars(i.e., : and /). Like the real - // wget, we leave query string or hash portions in tact. - var path = options.filename || url.replace(/[:/]/, '').split('/').pop(); + // Grab whatever is after the last / (assuming there is one). Like the real + // wget, we leave query string or hash portions in tact. This assumes a + // properly encoded URL. + // i.e. instead of "/foo?bar/" we would expect "/foo?bar%2F" + var path = options.filename || url.split('/').pop(); + path = Path.resolve(fs.cwd, path); function onerror() { callback(new Error('unable to get resource')); } - Network.download('get', url, function(err, data) { + Network.download(url, function(err, data) { if (err || !data) { return onerror(); } @@ -467,7 +469,7 @@ define(function(require) { callback(null, path); } }); - } + }); }; Shell.prototype.unzip = function(zipfile, options, callback) { diff --git a/tests/require-config.js b/tests/require-config.js index 6fb739c..9178d05 100644 --- a/tests/require-config.js +++ b/tests/require-config.js @@ -80,6 +80,7 @@ var config = (function() { "tests": "../tests", "spec": "../tests/spec", "bugs": "../tests/bugs", + "src": "../src", "util": "../tests/lib/test-utils", "Filer": "../dist/filer-test" }, diff --git a/tests/spec/fs.open.spec.js b/tests/spec/fs.open.spec.js index 0dd4c87..e57a2bc 100644 --- a/tests/spec/fs.open.spec.js +++ b/tests/spec/fs.open.spec.js @@ -1,4 +1,4 @@ -define(["Filer", "util"], function(Filer, util) { +define(["Filer", "util", "src/constants"], function(Filer, util, constants) { describe('fs.open', function() { beforeEach(util.setup); @@ -81,7 +81,7 @@ define(["Filer", "util"], function(Filer, util) { it('should return the argument value of the file descriptor index matching the value set by the first useable file descriptor constant', function(done) { var fs = util.fs(); - var firstFD = require('src/constants').FIRST_DESCRIPTOR; + var firstFD = constants.FIRST_DESCRIPTOR; var fd1; fs.open('/file1', 'w+', function(error, fd) { diff --git a/tests/spec/lib.spec.js b/tests/spec/lib.spec.js index 93b76ee..ede6acc 100644 --- a/tests/spec/lib.spec.js +++ b/tests/spec/lib.spec.js @@ -1,40 +1,62 @@ define(['../../src/network'], function(network) { describe('Network download tool', function() { - if(typeof XMLHttpRequest === "undefined") { - it('should connect to server',function(done) { - var request = require('request'); - request({ - url: 'http://localhost:8080/package.json', - method: 'GET', - encoding: 'utf-8' - }, function(error, msg, body) { - expect(error).not.to.exist; - done(); - }); - }); + var uri; + + if (typeof XMLHttpRequest === 'undefined') { + // Node context + uri = { + valid: 'http://localhost:1234/package.json', + invalid: 'booyah!', + notFound: 'http://localhost:1234/this-isnt-real' + } + } else { + // Browser context + uri = { + valid: '../package.json', + invalid: 'asdf://booyah!', + notFound: 'this-isnt-real' + }; } it('should throw an exception when a URI is not passed', function(done) { expect(function() { - network.download(undefined, function(error, data, statusCode) {}); + network.download(undefined, function(error, data) {}); }).to.throwError; done(); }); it('should get an error when a non-existent path is specified', function(done) { - network.download('http://localhost:8080/file-non-existent', function(error, data, statusCode) { - expect(error).not.to.exist; - expect(statusCode).to.eql('404'); - expect(data).to.be.a(null); + network.download(uri.notFound, function(error, data) { + expect(error).to.exist; + expect(error.code).to.eql(404); + expect(data).to.be.eql(null); done(); }); }); + if (typeof XMLHttpRequest === 'undefined') { + it('in nodejs, should get an error when an invalid URI is specified', function(done) { + network.download(uri.invalid, function(error, data) { + expect(error).to.exist; + expect(error.code).to.eql(null); + expect(error.message).to.exist; + expect(data).to.be.eql(null); + done(); + }); + }); + } else { + it('in a browser, should throw an error when an invalid URI is specified', function(done) { + expect(function(){ + network.download(uri.invalid, function() {}); + }).to.throwError; + done(); + }); + } + it('should download a resource from the server', function(done) { - network.download('http://localhost:8080/package.json', function(error, data, statusCode) { + network.download(uri.valid, function(error, data) { expect(error).not.to.exist; - expect(statusCode).to.eql('200'); expect(data).to.exist; expect(data).to.have.length.above(0); done(); diff --git a/tests/spec/node-js/simple/nodejs.spec.js b/tests/spec/node-js/simple/nodejs.spec.js new file mode 100644 index 0000000..3fde918 --- /dev/null +++ b/tests/spec/node-js/simple/nodejs.spec.js @@ -0,0 +1,13 @@ +define(["Filer", "util"], function(Filer, util) { + + describe('Nodejs compatability', function() { + beforeEach(util.setup); + afterEach(util.cleanup); + + it('module should be requireable', function() { + expect(function() { + var Filer = require('../../dist/filer_node.js'); + }).to.not.throwError; + }); + }); +}); diff --git a/tests/spec/shell/wget.spec.js b/tests/spec/shell/wget.spec.js index 078c04a..dfc7387 100644 --- a/tests/spec/shell/wget.spec.js +++ b/tests/spec/shell/wget.spec.js @@ -34,7 +34,7 @@ define(["Filer", "util"], function(Filer, util) { it('should download the contents of a file from a url to default filename', function(done) { var fs = util.fs(); var shell = fs.Shell(); - var url = "test-file.txt"; + var url = typeof XMLHttpRequest === "undefined" ? "http://localhost:1234/tests/test-file.txt" : "/tests/test-file.txt"; var contents = "This is a test file used in some of the tests.\n"; shell.wget(url, function(err, path) { @@ -54,7 +54,7 @@ define(["Filer", "util"], function(Filer, util) { it('should download the contents of a file from a url to specified filename', function(done) { var fs = util.fs(); var shell = fs.Shell(); - var url = "test-file.txt"; + var url = typeof XMLHttpRequest === "undefined" ? "http://localhost:1234/tests/test-file.txt" : "/tests/test-file.txt"; var contents = "This is a test file used in some of the tests.\n"; shell.wget(url, { filename: 'test-file.txt' }, function(err, path) { @@ -74,7 +74,7 @@ define(["Filer", "util"], function(Filer, util) { it('should download the contents of a file from a url with query string', function(done) { var fs = util.fs(); var shell = fs.Shell(); - var url = "test-file.txt?foo"; + var url = typeof XMLHttpRequest === "undefined" ? "http://localhost:1234/tests/test-file.txt?foo" : "/tests/test-file.txt?foo"; var contents = "This is a test file used in some of the tests.\n"; shell.wget(url, function(err, path) { @@ -91,26 +91,6 @@ define(["Filer", "util"], function(Filer, util) { }); }); - it('should download the contents of a file from a url to specified filename, stripping : and /', function(done) { - var fs = util.fs(); - var shell = fs.Shell(); - var url = "test-file.txt?foo=:/"; - var contents = "This is a test file used in some of the tests.\n"; - - shell.wget(url, function(err, path) { - if(err) throw err; - - expect(path).to.equal('/test-file.txt?foo='); - - fs.readFile(path, 'utf8', function(err, data) { - if(err) throw err; - - expect(data).to.equal(contents); - done(); - }); - }); - }); - }); }); diff --git a/tests/spec/shell/zip-unzip.spec.js b/tests/spec/shell/zip-unzip.spec.js index 1e70d72..95566a1 100644 --- a/tests/spec/shell/zip-unzip.spec.js +++ b/tests/spec/shell/zip-unzip.spec.js @@ -1,14 +1,5 @@ define(["Filer", "util"], function(Filer, util) { - // PhantomJS doesn't like how the zlib stuff works, see: - // https://github.com/imaya/zlib.js/issues/33 - function itShouldWorkInPhantomJSButDoesNot(test, callback) { - if(navigator.userAgent.indexOf('PhantomJS') > -1) { - return it.skip(test, callback); - } - it(test, callback); - } - describe('FileSystemShell.zip() and unzip()', function() { beforeEach(util.setup); afterEach(util.cleanup); @@ -29,16 +20,17 @@ define(["Filer", "util"], function(Filer, util) { }); }); - itShouldWorkInPhantomJSButDoesNot('should download and unzip the contents of a zip file', function(done) { + it('should download and unzip the contents of a zip file', function(done) { var fs = util.fs(); var shell = fs.Shell(); - var url = "test-file.txt.zip"; + var url = typeof XMLHttpRequest === "undefined" ? "http://localhost:1234/tests/test-file.txt.zip" : "/tests/test-file.txt.zip"; + var filename = "test-file.txt.zip"; var contents = "This is a test file used in some of the tests.\n"; fs.writeFile('/original', contents, function(err) { if(err) throw err; - shell.wget(url, {filename: url}, function(err, path) { + shell.wget(url, {filename: filename}, function(err, path) { if(err) throw err; shell.unzip(path, function(err) { @@ -59,17 +51,18 @@ define(["Filer", "util"], function(Filer, util) { }); }); - itShouldWorkInPhantomJSButDoesNot('should download and unzip the contents of a zip file with a specified destination', function(done) { + it('should download and unzip the contents of a zip file with a specified destination', function(done) { var fs = util.fs(); var shell = fs.Shell(); var Path = Filer.Path; - var url = "test-file.txt.zip"; + var url = typeof XMLHttpRequest === "undefined" ? "http://localhost:1234/tests/test-file.txt.zip" : "/tests/test-file.txt.zip"; + var filename = "test-file.txt.zip"; var contents = "This is a test file used in some of the tests.\n"; fs.writeFile('/original', contents, function(err) { if(err) throw err; - shell.wget(url, {filename: url}, function(err, path) { + shell.wget(url, {filename: filename}, function(err, path) { if(err) throw err; shell.tempDir(function(err, tmp) { @@ -93,7 +86,7 @@ define(["Filer", "util"], function(Filer, util) { }); }); - itShouldWorkInPhantomJSButDoesNot('should be able to zip and unzip a file', function(done) { + it('should be able to zip and unzip a file', function(done) { var fs = util.fs(); var file = '/test-file.txt'; var zipfile = file + '.zip'; @@ -134,7 +127,7 @@ define(["Filer", "util"], function(Filer, util) { }); }); - itShouldWorkInPhantomJSButDoesNot('should be able to handle a deep tree structure in a zip', function(done) { + it('should be able to handle a deep tree structure in a zip', function(done) { // test-dir.zip has the following structure: // // test-dir/ @@ -145,7 +138,8 @@ define(["Filer", "util"], function(Filer, util) { var fs = util.fs(); var shell = fs.Shell(); - var url = "test-dir.zip"; + var url = typeof XMLHttpRequest === "undefined" ? "http://localhost:1234/tests/test-dir.zip" : "/tests/test-dir.zip"; + var filename = "test-dir.zip"; var Path = Filer.Path; var contents = "This is a test file used in some of the tests.\n"; @@ -175,7 +169,7 @@ define(["Filer", "util"], function(Filer, util) { }); } - shell.wget(url, {filename: url}, function(err, path) { + shell.wget(url, {filename: filename}, function(err, path) { if(err) throw err; shell.unzip(path, function(err) { @@ -194,7 +188,7 @@ define(["Filer", "util"], function(Filer, util) { }); }); - itShouldWorkInPhantomJSButDoesNot('should be able to re-create (unzip/zip) a deep tree structure in a zip', function(done) { + it('should be able to re-create (unzip/zip) a deep tree structure in a zip', function(done) { // test-dir.zip has the following structure: // // test-dir/ @@ -205,7 +199,8 @@ define(["Filer", "util"], function(Filer, util) { var fs = util.fs(); var shell = fs.Shell(); - var url = "test-dir.zip"; + var url = typeof XMLHttpRequest === "undefined" ? "http://localhost:1234/tests/test-dir.zip" : "/tests/test-dir.zip"; + var filename = "test-dir.zip"; var Path = Filer.Path; var contents = "This is a test file used in some of the tests.\n"; @@ -235,7 +230,7 @@ define(["Filer", "util"], function(Filer, util) { }); } - shell.wget(url, {filename: url}, function(err, path) { + shell.wget(url, {filename: filename}, function(err, path) { if(err) throw err; shell.unzip(path, function(err) { @@ -266,7 +261,7 @@ define(["Filer", "util"], function(Filer, util) { }); }); - itShouldWorkInPhantomJSButDoesNot('should fail if the zipfile already exists', function(done) { + it('should fail if the zipfile already exists', function(done) { var fs = util.fs(); var shell = fs.Shell(); var file = "/file"; diff --git a/tests/test-manifest.js b/tests/test-manifest.js index 740643b..9020697 100644 --- a/tests/test-manifest.js +++ b/tests/test-manifest.js @@ -68,6 +68,9 @@ define([ "spec/node-js/simple/test-fs-watch", "spec/node-js/simple/test-fs-watch-recursive", + // Nodejs compatibility + "spec/node-js/simple/nodejs.spec", + // Regressions, Bugs "bugs/issue105", "bugs/issue106"