Merge pull request #15 from sharelatex/fix-acceptance-tests

Fix acceptance tests for filestore, run on travis

services/filestore/.travis.yml

@@ -10,8 +10,12 @@ install:
     - npm install
     - grunt install
 
+before_script:
+    - grunt forever:app:start
+
 script:
     - grunt test:unit
+    - grunt test:acceptance
 
 services:
     - redis-server

services/filestore/Gruntfile.coffee

@@ -2,6 +2,11 @@ module.exports = (grunt) ->
 
 	# Project configuration.
 	grunt.initConfig
+		forever:
+			app:
+				options:
+					index: "app.js"
+
 		coffee:
 			server: 
 				expand: true,
@@ -73,9 +78,11 @@ module.exports = (grunt) ->
 	grunt.loadNpmTasks 'grunt-contrib-clean'
 	grunt.loadNpmTasks 'grunt-concurrent'
 	grunt.loadNpmTasks 'grunt-mocha-test'
+	grunt.loadNpmTasks 'grunt-forever'
 
 	grunt.registerTask "test:unit", ["coffee", "mochaTest:unit"]
 	grunt.registerTask "test:acceptence", ["coffee", "mochaTest:acceptence"]
+	grunt.registerTask "test:acceptance", ["test:acceptence"]
 
 	grunt.registerTask "ci", "test:unit"
 	grunt.registerTask 'default', ['coffee', 'concurrent']

services/filestore/app/coffee/FSPersistorManager.coffee

@@ -1,6 +1,8 @@
 logger = require("logger-sharelatex")
 fs = require("fs")
 LocalFileWriter = require("./LocalFileWriter")
+rimraf = require("rimraf")
+response = require ("response")
 
 filterName = (key) ->
   return key.replace /\//g, "_"
@@ -31,7 +33,11 @@ module.exports =
     sourceStream = fs.createReadStream "#{location}/#{filteredName}"
     sourceStream.on 'error', (err) ->
       logger.err err:err, location:location, name:name, "Error reading from file"
+      if err.code = 'ENOENT'
+        callback null, response().html('NoSuchKey: file not found\n')
+      else
         callback err
+    sourceStream.on 'readable', () ->
       callback null, sourceStream
 
 
@@ -47,20 +53,25 @@ module.exports =
     targetStream.on 'error', (err) ->
       logger.err err:err, location:location, key:filteredToName, "Error writing to file"
       callback err
+    targetStream.on 'finish', () ->
+      callback null
     sourceStream.pipe targetStream
 
   deleteFile: (location, name, callback)->
     filteredName = filterName name
     logger.log location:location, name:filteredName, "delete file"
     fs.unlink "#{location}/#{filteredName}", (err) ->
+      if err?
         logger.err err:err, location:location, name:filteredName, "Error on delete."
         callback err
+      else
+        callback()
 
   deleteDirectory: (location, name, callback = (err)->)->
-    filteredName = filterName name
+    filteredName = filterName name.replace(/\/$/,'')
-    fs.rmdir "#{location}/#{filteredName}", (err) ->
+    rimraf "#{location}/#{filteredName}", (err) ->
-      logger.err err:err, location:location, name:filteredName, "Error on rmdir."
+      if err?
-      if err and err.code != 'ENOENT' 
+        logger.err err:err, location:location, name:filteredName, "Error on rimraf rmdir."
         callback err
       else
         callback()

services/filestore/app/coffee/FileHandler.coffee

@@ -16,10 +16,10 @@ module.exports =
 			PersistorManager.sendStream bucket, key, stream, callback
 
 	deleteFile: (bucket, key, callback)->
-		convetedKey = KeyBuilder.getConvertedFolderKey(bucket, key)
+		convetedKey = KeyBuilder.getConvertedFolderKey(key)
 		async.parallel [
 			(done)-> PersistorManager.deleteFile bucket, key, done
-			(done)-> PersistorManager.deleteFile bucket, convetedKey, done
+			(done)-> PersistorManager.deleteDirectory bucket, convetedKey, done
 		], callback
 
 	getFile: (bucket, key, opts = {}, callback)->

services/filestore/package.json

@@ -21,6 +21,8 @@
     "node-uuid": "~1.4.1",
     "pngcrush": "0.0.3",
     "request": "2.14.0",
+    "response": "0.14.0",
+    "rimraf": "2.2.8",
     "settings-sharelatex": "git+https://github.com/sharelatex/settings-sharelatex.git#master",
     "stream-buffers": "~0.2.5",
     "underscore": "~1.5.2"
@@ -35,6 +37,7 @@
     "grunt-contrib-watch": "0.5.3",
     "grunt-nodemon": "0.2.1",
     "grunt-contrib-clean": "0.5.0",
-    "grunt-concurrent": "0.4.2"
+    "grunt-concurrent": "0.4.2",
+    "grunt-forever": "~0.4.4"
   }
 }

services/filestore/test/acceptence/coffee/SendingFileTest.coffee

@@ -71,13 +71,16 @@ describe "Filestore", ->
 			newFileId = Math.random()
 			newFileUrl = "#{@filestoreUrl}/project/#{newProjectID}/file/#{newFileId}"
 			opts =
+				method: 'put'
 				uri: newFileUrl
 				json:
 					source:
 						project_id:"acceptence_tests"
 						file_id: @file_id
-			request.put opts, (err)=>
+			request opts, (err, response, body)=>
+				response.statusCode.should.equal 200
 				request.del @fileUrl, (err, response, body)=>
+					response.statusCode.should.equal 204
 					request.get newFileUrl, (err, response, body)=>
 						body.should.equal @constantFileContent
 						done()

services/filestore/test/unit/coffee/FSPersistorManagerTests.coffee

@@ -6,6 +6,7 @@ expect = chai.expect
 modulePath = "../../../app/js/FSPersistorManager.js"
 SandboxedModule = require('sandboxed-module')
 fs = require("fs")
+response = require("response")
 
 describe "FSPersistorManagerTests", ->
 
@@ -17,6 +18,7 @@ describe "FSPersistorManagerTests", ->
       unlink:sinon.stub()
       rmdir:sinon.stub()
       exists:sinon.stub()
+    @Rimraf = sinon.stub()
     @LocalFileWriter =
       writeStream: sinon.stub()
     @requires =
@@ -25,6 +27,8 @@ describe "FSPersistorManagerTests", ->
       "logger-sharelatex":
         log:->
         err:->
+      "response":response
+      "rimraf":@Rimraf
     @location = "/tmp"
     @name1 = "530f2407e7ef165704000007/530f838b46d9a9e859000008"
     @name1Filtered ="530f2407e7ef165704000007_530f838b46d9a9e859000008"
@@ -115,11 +119,11 @@ describe "FSPersistorManagerTests", ->
 
   describe "deleteDirectory", ->
     beforeEach ->
-      @Fs.rmdir.callsArgWith(1,@error)
+      @Rimraf.callsArgWith(1,@error)
 
-    it "Should call rmdir with correct options", (done) ->
+    it "Should call rmdir(rimraf) with correct options", (done) ->
       @FSPersistorManager.deleteDirectory @location, @name1, (err) =>
-        @Fs.rmdir.calledWith("#{@location}/#{@name1}").should.equal.true
+        @Rimraf.calledWith("#{@location}/#{@name1}").should.equal.true
         done()
 
     it "Should propogate the error", (done) ->

services/filestore/test/unit/coffee/FileHandlerTests.coffee

@@ -69,6 +69,7 @@ describe "FileHandler", ->
 		beforeEach ->
 			@keyBuilder.getConvertedFolderKey.returns(@stubbedConvetedKey)
 			@PersistorManager.deleteFile.callsArgWith(2)
+			@PersistorManager.deleteDirectory.callsArgWith(2)
 
 		it "should tell the filestore manager to delete the file", (done)->
 			@handler.deleteFile @bucket, @key, =>
@@ -77,7 +78,7 @@ describe "FileHandler", ->
 
 		it "should tell the filestore manager to delete the cached foler", (done)->
 			@handler.deleteFile @bucket, @key, =>
-				@PersistorManager.deleteFile.calledWith(@bucket, @stubbedConvetedKey).should.equal true
+				@PersistorManager.deleteDirectory.calledWith(@bucket, @stubbedConvetedKey).should.equal true
 				done()
 
 	describe "getFile", ->