js-must icon indicating copy to clipboard operation
js-must copied to clipboard

Unexpected behavior with undefined

Open capelio opened this issue 7 years ago • 5 comments

Using 0.13.4:

// fails as expected
expect([]).to.not.be.empty()
expect(null).to.not.be.empty()

// passes unexpectedly
expect(undefined).to.not.be.empty()

Does must.js treat undefined as a distinct case? In the above cases, I'd expect it to fail like null and [], especially considering the frequency of x is undefined errors in Javascript due to incorrect object properties being accessed, etc.

Appreciate all the work you're doing moll! :)

capelio avatar Feb 19 '17 22:02 capelio

Hey there! Thanks for bringing this lack of empty's clarity up.

My intention with empty was for it to only be used only with strings, arrays or objects. I guess I didn't bother throwing any errors for nulls or undefineds because I was presuming undefined.must.be.empty() would suffice as an error. However, empty doesn't raise an error when the wrapper approach is taken, like your examples above. I'm not sure whether to throw an error for inputs for which asking "is it empty" doesn't really make sense or assume they're all empty...

moll avatar Feb 19 '17 22:02 moll

Definitely see where your thought process is coming from, and it makes sense. It's possible the way in which I use Must.js is a bit atypical. In my mind, I'd expect Must to lean toward accommodating the reality that, in Javascript, despite one's best efforts there's no way to guarantee that something will only ever be a particular type or types.

Given the patterns I'm using today, I could address this with something like:

const somePromise = promiseReturningMethod()
  .then(someArray => {
    // I'd really rather prefer to keep logic like this out of tests
    someArray = someArray || []

    expect(someArray).to.not.be.empty()

    someArray.forEach(element => {
      // Ensure each array element is an object with particular keys
      expect(element).to.have.keys(['foo', 'bar'])
    })

    return someArray
  })

expect(somePromise)
  .to.resolve
  .to.be.an.array()

What about adding some type of if (!isEmptyable(thing)) check to Must internals in scenarios where it wants to only apply an assertion against a whitelist of types?

capelio avatar Feb 19 '17 23:02 capelio

Are you basically checking whether that promise resolves to either undefined or null OR to an array matching some other requirements?

moll avatar Feb 20 '17 09:02 moll

I'm attempting to ensure the promise resolves to an array of objects, each of which has a set of keys. The test should fail in all other cases (promise rejection, returning undefined, null, an object, a string, etc).

On Feb 20, 2017 4:03 AM, "Andri Möll" [email protected] wrote:

Are you basically checking whether that promise resolves to either undefined or null OR to an array matching some other requirements?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/moll/js-must/issues/58#issuecomment-281022983, or mute the thread https://github.com/notifications/unsubscribe-auth/AAjBxdum2_TKpGu2TXBgjo-z0G-xNCKiks5reVbMgaJpZM4MFnU- .

capelio avatar Feb 20 '17 14:02 capelio

If you don't mind, I'd like to keep this open as I think something should be done to handle the empty check on non-empty values. Apologies for not having gotten to it earlier though.

moll avatar Aug 23 '19 08:08 moll