Uploaded image for project: 'Couchbase node.js Client Library'
  1. Couchbase node.js Client Library
  2. JSCBC-1189

Fix crud tests that have callback

    XMLWordPrintable

Details

    • Task
    • Resolution: Resolved
    • Major
    • 4.2.8
    • None
    • None
    • None

    Description

      Right now some crud tests are failing because the options block is using a 5ms timeout which is too short for the test to complete. For these tests we don't actually care about the timeout value, we just want an options block, but the timeout should not actually cause a timeout.

      Impacted tests:
      #crud #basic #exists.should perform basic exists with options and callback
      #crud #basic #remove.should perform basic remove with options and callback
      #crud #binary #increment.should increment successfully with options and callback
      #crud #binary #decrement.should decrement successfully with options and callback
      #crud #binary #prepend.should prepend successfully with options and callback

      Also, I noticed that tests with callbacks are structured incorrectly so if there is a problem w/ the test the end result looks to be a timeout from mocha's perspective. An example is below on how to restructure the tests that are using a callback.

      Example of test when we expect an error:

            it('should perform errored exists with callback', function (callback) {
              collFn().get('a-missing-key', (err, res) => {
                res
                assert.isOk(err)
                callback(null)
              })
            })
      

      to something like the following:

            it('should perform errorred exists with callback', function (done) {
              collFn().get('a-missing-key', (err, res) => {
                try {
                  assert.isNull(res)
                  assert.isOk(err)
                  done(null)
                }catch(e){
                  done(e)
                }
              })
            })
      

      Example of test when an error is not expected:

            it('should perform basic exists with options and callback', function (callback) {
              collFn().exists(testKeyA, { timeout: 5 }, (err, res) => {
                assert.isObject(res)
                assert.strictEqual(res.exists, true)
                callback(err)
              })
            })
      

      to something like the following:

            it('should perform basic exists with options and callback', function (done) {
              collFn().exists(testKeyA, { timeout: 5 }, (err, res) => {
                if(err){
                  return done(err)
                }
                try {
                  assert.isObject(res)
                  assert.strictEqual(res.exists, true)
                  done(null)
                }catch(e){
                  done(e)
                }
              })
            })
      

      Of course there is some flexibility to how assertions are made, but the general path should be to wrap the assertions in a try/catch so that the appropriate error can be passed along. Also, as in the latter example, we could assert that the error is null instead of checking and returning...return does give us better info on what the error was though.

      Also, while the name of the callback does not technically matter, I wonder if using `done` would be better as that seems to be idiomatic to the mocha test framework.

      Attachments

        Issue Links

          No reviews matched the request. Check your Options in the drop-down menu of this sections header.

          Activity

            People

              matt.wozakowski Matt Wozakowski
              jared.casey Jared Casey
              Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Gerrit Reviews

                  There are no open Gerrit changes

                  PagerDuty