Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unexpected behavior with undefined #58

Open
capelio opened this issue Feb 19, 2017 · 5 comments
Open

Unexpected behavior with undefined #58

capelio opened this issue Feb 19, 2017 · 5 comments

Comments

@capelio
Copy link

capelio commented Feb 19, 2017

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
Copy link
Owner

moll commented Feb 19, 2017

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
Copy link
Author

capelio commented Feb 19, 2017

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
Copy link
Owner

moll commented Feb 20, 2017

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

@capelio
Copy link
Author

capelio commented Feb 20, 2017 via email

@capelio capelio closed this as completed Aug 22, 2019
@moll
Copy link
Owner

moll commented Aug 23, 2019

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 moll reopened this Aug 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants