search

mozilla / makedrive

[RETIRED] Webmaker Filesystem
Mozilla Public License 2.0
351 stars 33 forks source link

Fix #308 - Configure makedrive to sync on a per file basis #458

Closed gideonthomas closed 9 years ago

gideonthomas commented 9 years ago

@humphd I'm having an issue with the fs-utils cleanup that I did. I keep getting an Object #<Object> has no method 'queueOrRun' error for many tests for fs-Utils. Here's the stack trace:

 at FileSystem.(anonymous function) (/Users/gideonthomas/makedrive/node_modules/filer/src/filesystem/interface.js:302:20)
      at getAttr (/Users/gideonthomas/makedrive/lib/fs-utils.js:50:3)
      at Object.fgetPartial (/Users/gideonthomas/makedrive/lib/fs-utils.js:155:3)
      at FileSystem.<anonymous> (/Users/gideonthomas/makedrive/tests/unit/fs-utils.js:299:17)
      at complete (/Users/gideonthomas/makedrive/node_modules/filer/src/filesystem/interface.js:315:18)
      at Object.complete [as _onImmediate] (/Users/gideonthomas/makedrive/node_modules/filer/src/filesystem/implementation.js:79:5)
      at processImmediate [as _immediateCallback] (timers.js:345:15)

I feel like I'm missing something coz it seems that fs seems to change when I call getAttr() and I'm not sure how.

humphd commented 9 years ago

When you do this var getFn = fs.fgetxattr; you are pulling the function fgetxattr off the fs object. If you then wanted to call that function using fs as the owning object, you'd need to use fgetxattr.call(fs, ...);.

A better way to do it is to store the name of the function to call, so you can just do fs[methodName](...);.

gideonthomas commented 9 years ago

@humphd just wanted to give a quick progress update since I haven't in a while.

I am at the point where I'm fixing tests now. After I finish fixing the existing tests (current integration test cases seem to fit in fine), I will add some more unit tests to cover per file specific test cases. This should be done hopefully soon and should be up for review.

gideonthomas commented 9 years ago

@alicoding @humphd I could use a little help fixing a test: The autosync tests in tests/unit/client/auto-syncing.js fail due to a bizzare error. It fails at:

sync.once('disconnected', function onDisconnected() {
        util.deleteFilesystemLayout(fs, null, function(err) {
          expect(err).not.to.exist;

because I get err as ENOENT: /file1 does not exist

I dug around into that function but couldn't find what was wrong because I was able to do a readFile in the function and not get an error but when the sh.rm executes on the same path, it returns an error.

I then went into filer and looked around fs.unlink and each step seems to execute properly until it reaches the final step, update_node_times() where the callback is called WITH NO ERROR. But that callback is in interface.js in a function called complete() which somehow magically has an argument as an error (even though it wasn't passed in through the callback in update_node_times).

Not sure how this is happening so I am moving onto other tests. Any help would be greatly appreciated.

gideonthomas commented 9 years ago

It seems like the same problem occurs not just with fs.unlink but also with fs.rmdir. No idea whats going on.

gideonthomas commented 9 years ago

@humphd I think I'm gonna re-write the tests in socket-tests.js and helper methods in util.js because a lot of the code is really confusing and there are so many instances of mixing async with sync code. Its making my work fixing the tests really hard.