vot / dbclone

Utility to move data between NoSQL databases
https://www.npmjs.com/package/dbclone
MIT License
3 stars 8 forks source link

Create unit tests #18

Open anishkny opened 5 years ago

anishkny commented 5 years ago

Now that test framework is scaffolded out in #17, should add some tests 😃

https://github.com/vot/dbclone/blob/8e0522f1b187fb4b31882be3a6bd2beaea2a6be4/test/lib.test.js#L6

I tried to decipher code structure a little bit but now sure how to go about it. Would be great if I got some help to start?

vot commented 5 years ago

@anishkny Sounds good, happy to see your continued involvement :)

I'll try to provide a few pointers shortly but here are the highlights:

anishkny commented 5 years ago

@anishkny Sounds good, happy to see your continued involvement :)

I am just doing this for fun, my commits might be sporadic, hope thats okay! 😃

  • I guess we may want to spin up a Mongo container somewhere during build (can't remember if that can we do that in Circle - worth checking) or provide an external Mongo to dump the data into during tests (mLab / Atlas / some other free option)

CircleCI supports Mongo via Docker. I have it working here.

  • a good place to start would be installing dbclone globally and exporting some Mongo database - that'll give you the files in data directory (within your working dir). It's all just JSON structure with annotated data types along the values - take a look at what it produces to understand the process better :)

I was trying to export an mLab Mongo I have but couldn't figure out how to pass in my password? If you could have this example in README it would help others too!

vot commented 5 years ago

@anishkny

Sporadic commits are absolutely fine - no particular pressure in terms of timing. Any help is welcome - the project is quite fun to write and it's somewhat contained in its scope. No need for it to grow rapidly.

I'd like this to be a well thought-through utility so that me and other people can save some time in the long run (even if it takes a while to get everything working perfectly).

Docker container for Mongo - nice one, didn't remember whether that was a thing in Circle. If we can just spin up one for testing that's ideal.

Auth - you can actually provide a fully qualified Mongo URI as host option, i.e. dbclone export --host mongodb://username:p4ssw0rd@ds111129.mlab.com:11129/my-database --db my-database

anishkny commented 5 years ago

For some reason, in my test, the callback of export receives undefined for exportErr, exportData.

dbclone.export({
  host: TEST_MLAB_URL,
  db: getDBNameFromURL(TEST_MLAB_URL),
  dataDir: DATADIR,
}, (e, d) => {
  console.log(`e = [${e}]`);      // <--- undefined
  console.log(`d = [${d}]`);      // <--- undefined
  resolve();
});

See: https://github.com/anishkny/dbclone/blob/3c83e3ece838cb6e72ee1456cf87a3252689cb31/test/lib.test.js#L28

Run in CI: https://circleci.com/gh/anishkny/dbclone/15

Rest of it seems fine - data is exported to JSON but callback does not get correct arguments.

vot commented 5 years ago

Hi @anishkny,

Seems like we're not returning anything in the callback of export method (see line 89 of lib/export.js).

Errors would be returned just fine but we don't pass anything when everything went well - probably a summary of what's been exported would be useful there.

This is what I'm thinking should be the data returned:

{
  "system.indexes": 1,
  "bios": 10
}

Does that seem like something you'd expect returned from that method?

anishkny commented 5 years ago

TBH I wasn't sure what to expect, just not undefined. This was my thinking based on the README example:

dbclone.export(exportOpts, (exportErr, exportData) => {

  // in the happy case: exportErr=null, exportData={something useful?}

  // in the error case: exportErr=an_Error_object, exportData=null/undefined

}
vot commented 5 years ago

Agreed - your expectation of "something useful" is absolutely correct.

I think if we simply list the collections and the number of documents within them then that would be the most useful thing at that point of the process.

anishkny commented 5 years ago

👍

vot commented 5 years ago

Hi @anishkny,

This is now resolved as part of #22.

The interfaces have now have much clearer boundaries between them and the data is passed through callbacks as it should be :)

They're now returning an array of objects with collection and documents properties (collection name and document count respectively). Exception to this is the drop method which just returns a boolean to indicate success of the operation.