denoland / std

The Deno Standard Library
https://jsr.io/@std
MIT License
3.14k stars 617 forks source link

testing/asserts : `assertFalse` #2045

Closed UltiRequiem closed 2 years ago

UltiRequiem commented 2 years ago

Is your feature request related to a problem? Please describe.

Currently, the testing module lacks a method to check if a value is False.

Describe the solution you'd like

export function assertFalse(expr: unknown, msg = ""): asserts expr is false {
  if (expr) {
    throw new AssertionError(msg);
  }
}

Describe alternatives you've considered

Deno.test("Text with letter is not numeric.", () => {
  assertEquals(isNumber("hello"), false);
  assert(isNumber("hello") === false);
  assert(!isNumber("hello"));
});

With the solution shown before we could

Deno.test("Text with letter is not numeric.", () => {
  assertFalse(isNumber("hello"));
});

Which I think is a lot more readable. Similar to 👇🏽 https://jestjs.io/docs/expect#tobefalsy https://github.com/avajs/ava/blob/main/docs/03-assertions.md#falseactual-message

getspooky commented 2 years ago

@UltiRequiem you can use assert or assertEquals so no need to add assertFalse

UltiRequiem commented 2 years ago

@UltiRequiem you can use assert or assertEquals so no need to add assertFalse

I already listed both options on Describe alternatives you've considered.

Despite that, an assertFalse function would upgrade readability and other libraries include it.

UltiRequiem commented 2 years ago

Another library that has something similar is Ava.

https://github.com/avajs/ava/blob/main/docs/03-assertions.md#falseactual-message

kt3k commented 2 years ago

This looks too trivial to include in std to me.

Is there any supportive opinions to this other than the above?

UltiRequiem commented 2 years ago

This looks too trivial to include in std to me.

Is there any supportive opinions to this other than the above?

I think the two strongest points are readability, and that other frameworks have it.

But I understand if it is not wanted here 😆

bcheidemann commented 2 years ago

I don't have a strong opinion if this should be implemented or not.

However, it should be assertNot in my opinion to match the existing convention set by other assertions e.g. assertEquals() vs assertNotEquals().

bcheidemann commented 2 years ago

@bartlomieju @kt3k can we make a call yet on if we want to include this or not? The reasons for including it seem valid to me and I know I (and many other developers) would use it even though it is trivial 😉

bartlomieju commented 2 years ago

If there's prior art for such assertion I'm willing to squint my eyes and accept it.

kt3k commented 2 years ago

Researched other languages and libraries. Some languages seem intentionally avoiding including it (Node, Ruby, Rust, etc), but many languages/libraries seem having assertFalse or similar. Interestingly I couldn't find any example of assertNot, which I personally think is worth considering.

Libs/Languages which have something similar to assertFalse

Libs/Languages which don't have something similar to assertFalse

bcheidemann commented 2 years ago

@kt3k That's very interesting 🤔 seems like a fairly even split.

I think assertFalse makes sense for many of the other languages mentioned because they have much narrower definitions of what it means to be falsy.

In JS I think it is misleading because assertFalse(""); assertFalse(0); assertFalse(null); assertFalse(undefined); assertFalse(NaN); would all pass.

I suggested assertNot because it fits the pattern for inverse assertions in deno_std but I think assertFalsy would also make sense. This would match Jest which has expect.toBeFalsy.

I think Jest also has toBeTrue and toBeFalse but these match only the literal values true and false.

bartlomieju commented 2 years ago

@kt3k since there are more examples of support for this feature I think we can add it.

UltiRequiem commented 2 years ago

Closing because the feature was merged.