moll / js-must

An assertion library for JavaScript and Node.js with a friendly BDD syntax (awesome.must.be.true()). It ships with many expressive matchers and is test runner and framework agnostic. Follows RFC 2119 with its use of MUST. Good stuff and well tested.
Other
336 stars 35 forks source link

Unexpected behavior with undefined #58

Open capelio opened 7 years ago

capelio commented 7 years ago

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! :)

moll commented 7 years ago

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...

capelio commented 7 years ago

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?

moll commented 7 years ago

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

capelio commented 7 years ago

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" notifications@github.com 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- .

moll commented 5 years ago

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.