Fix issue #286 - Renaming a file end up with two copies

CONTRIBUTING.md

@@ -84,8 +84,6 @@ You can run the tests in a nodejs context by running `grunt test`.
 If you're writing tests, make sure you write them in the same style as existing tests. See `tests/lib/util.js` and how it gets used in various tests as
 an example.
 
-**NOTE:** *The entire test suite has been updated to be parsed by JSHint, so please make sure to prepend any newly added test files with* `/*jshint expr: true*/`.
-
 ## Communication
 
 If you'd like to talk to someone about the project, you can reach us on irc.mozilla.org in the #nimble or #mofodev channel. Look for "aali" or "humph".

Gruntfile.js

@@ -151,13 +151,15 @@ module.exports = function(grunt) {
     },
 
     jshint: {
+      options: {
+        expr: true
+      },
       all: [
         "Gruntfile.js",
         "client/src/**/*.js",
         "server/**/*.js",
         "lib/**/*.js",
-        "tests/unit/**/*.js",
-        "tests/lib/*.js"
+        "tests/**/**/*.js"
       ]
     },
 

client/src/sync-filesystem.js

@@ -98,10 +98,15 @@ function SyncFileSystem(fs) {
   });
 
   // Wrapped fs methods that have path at second arg position
-  ['rename', 'link', 'symlink'].forEach(function(method) {
+  ['link', 'symlink'].forEach(function(method) {
     self[method] = wrapMethod(method, 1, setUnsynced);
   });
 
+  // Wrapped fs methods that have path at second arg position, and need to use the parent path.
+  ['rename'].forEach(function(method) {
+    self[method] = wrapMethod(method, 1, setUnsynced, true);
+  });
+
   // Wrapped fs methods that use file descriptors
   ['ftruncate', 'futimes', 'write'].forEach(function(method) {
     self[method] = wrapMethod(method, 0, fsetUnsynced);

tests/integration/sync-tests.js

@@ -19,10 +19,8 @@ describe('Two clients', function(){
   it('should be able to sync two files in one direction', function(done) {
     var file1 = {'/dir/file1.txt': 'This is file 1'};
     var file2 = {'/dir/file2.txt': 'This is file 2'};
-    var finalLayout = {};
-
-    for(var k in file1) finalLayout[k] = file1[k];
-    for(var k in file2) finalLayout[k] = file2[k];
+    var finalLayout = { '/dir/file1.txt': 'This is file 1',
+                        '/dir/file2.txt': 'This is file 2' };
 
     util.authenticatedConnection(function(err, result1) {
       expect(err).not.to.exist;

tests/lib/util.js

@@ -1,5 +1,3 @@
-/*jshint expr: true*/
-
 var request = require('request');
 var expect = require('chai').expect;
 var ws = require('ws');

tests/unit/bugs.js

@@ -1,5 +1,3 @@
-/*jshint expr: true*/
-
 var expect = require('chai').expect;
 var util = require('../lib/util.js');
 var SyncMessage = require('../../lib/syncmessage');

tests/unit/client/auto-syncing.js

@@ -1,5 +1,3 @@
-/*jshint expr: true*/
-
 var expect = require('chai').expect;
 var util = require('../../lib/util.js');
 var MakeDrive = require('../../../client/src');

tests/unit/client/conflict.js

@@ -1,5 +1,3 @@
-/*jshint expr: true*/
-
 var expect = require('chai').expect;
 var util = require('../../lib/util.js');
 var MakeDrive = require('../../../client/src');

tests/unit/client/deep-and-wide.js

@@ -1,5 +1,3 @@
-/*jshint expr: true*/
-
 var expect = require('chai').expect;
 var util = require('../../lib/util.js');
 var MakeDrive = require('../../../client/src');

tests/unit/client/deep-tree.js

@@ -1,5 +1,3 @@
-/*jshint expr: true*/
-
 var expect = require('chai').expect;
 var util = require('../../lib/util.js');
 var MakeDrive = require('../../../client/src');

tests/unit/client/empty-dir.js

@@ -1,5 +1,3 @@
-/*jshint expr: true*/
-
 var expect = require('chai').expect;
 var util = require('../../lib/util.js');
 var MakeDrive = require('../../../client/src');

tests/unit/client/large-files.js

@@ -1,5 +1,3 @@
-/*jshint expr: true*/
-
 var expect = require('chai').expect;
 var util = require('../../lib/util.js');
 var MakeDrive = require('../../../client/src');

tests/unit/client/makedrive-api.js

@@ -1,5 +1,3 @@
-/*jshint expr: true*/
-
 var expect = require('chai').expect;
 var util = require('../../lib/util.js');
 var MakeDrive = require('../../../client/src');

tests/unit/client/many-small-files.js

@@ -1,5 +1,3 @@
-/*jshint expr: true*/
-
 var expect = require('chai').expect;
 var util = require('../../lib/util.js');
 var MakeDrive = require('../../../client/src');

tests/unit/client/multiple-file-sync.js

@@ -1,5 +1,3 @@
-/*jshint expr: true*/
-
 var expect = require('chai').expect;
 var util = require('../../lib/util.js');
 var MakeDrive = require('../../../client/src');

tests/unit/client/sync-filesystem.js

@@ -1,5 +1,3 @@
-/*jshint expr: true*/
-
 var expect = require('chai').expect;
 var util = require('../../lib/util.js');
 var MakeDrive = require('../../../client/src');

tests/unit/client/unsynced-attr-tests.js

@@ -1,5 +1,3 @@
-/*jshint expr: true*/
-
 var expect = require('chai').expect;
 var util = require('../../lib/util.js');
 var MakeDrive = require('../../../client/src');

tests/unit/fs-utils.js

@@ -1,5 +1,3 @@
-/*jshint expr: true*/
-
 var expect = require('chai').expect;
 var util = require('../lib/util.js');
 var MakeDrive = require('../../client/src');

tests/unit/http-routes-tests.js

@@ -1,5 +1,3 @@
-/*jshint expr: true*/
-
 var expect = require('chai').expect;
 var request = require('request');
 var util = require('../lib/util');

tests/unit/rename.js

@@ -0,0 +1,35 @@
+var expect = require('chai').expect;
+var util = require('../lib/util.js');
+var MakeDrive = require('../../client/src');
+
+describe('Renaming a file', function(){
+  it('should be able to rename and end up with single file after renamed', function(done) {
+    var originalLayout = {'/dir/file.txt': 'This is file 1'};
+    var newLayout = {'/dir/newFile.txt': 'This is file 1'};
+
+    util.setupSyncClient({layout: originalLayout, manual: true}, function(err, client) {
+      expect(err).not.to.exist;
+
+      var fs = client.fs;
+      var sync = client.sync;
+
+      fs.rename('/dir/file.txt', '/dir/newFile.txt', function(err) {
+        expect(err).not.to.exist;
+
+        client.sync.once('completed', function after() {
+          util.ensureRemoteFilesystem(newLayout, client.jar, function(err) {
+            expect(err).not.to.exist;
+
+            client.sync.on('disconnected', done);
+            client.sync.disconnect();
+          });
+        });
+        // FIXME: This should not be setTimeout.
+        // Change this after https://github.com/mozilla/makedrive/issues/287 fixed.
+        setTimeout(function() {
+          client.sync.request();
+        }, 1000);
+      });
+    });
+  });
+});

tests/unit/rsync-tests.js

@@ -1,5 +1,3 @@
-/*jshint expr: true*/
-
 var OPTION_SIZE = { size: 5 };
 var OPTION_REC_SIZE = { recursive: true, size: 5 };
 var CHUNK_SIZE = OPTION_SIZE.size;

tests/unit/socket-tests.js

@@ -1,5 +1,3 @@
-/*jshint expr: true*/
-
 var expect = require('chai').expect;
 var util = require('../lib/util.js');
 var SyncMessage = require('../../lib/syncmessage');

tests/unit/sync-path-resolver-tests.js

@@ -1,5 +1,3 @@
-/*jshint expr: true*/
-
 var expect = require('chai').expect;
 var pathResolver = require('../../lib/sync-path-resolver.js');
 var resolvePath = pathResolver.resolve;

tests/unit/util-tests.js

@@ -1,5 +1,3 @@
-/*jshint expr: true*/
-
 var expect = require('chai').expect;
 var util = require('../lib/util.js');
 var request = require('request');