alsatian icon indicating copy to clipboard operation
alsatian copied to clipboard

Add conditional negation operator.

Open cdibbs opened this issue 6 years ago • 6 comments

Hey, all! I have encountered a number of cases where I would love to combine two tests into one via @TestCase(...) but am prevented from expressing that succinctly due to the inability to negate an assertion, conditionally. At best, if you combine tests with TestCase, you end up with:

if (! shouldThrow) {
  Expect(func).not.toThrow();
} else {
  Expect(func).toThrow();
}

Am I missing a better pattern? If not, it seems to make sense to add a version of not that takes a boolean parameter for whether to do the negation.

Expect(func).maybe(shouldThrow).toThrow();
Expect(func).when(shouldThrow).toThrow();
Expect(func).iff(shouldThrow).toThrow();

In the fluent assertions extension I'm working on, I have already added a maybe operator, but am considering renaming it to when. It seems to work well, so far.

Assert(func).maybe(shouldThrow).throws();
Assert(func).when(shouldThrow).throws();
Assert(func).iff(shouldThrow).throws();

cdibbs avatar Apr 30 '18 17:04 cdibbs

Interesting problem @cdibbs - do you have some sample tests so I can understand this in context?

jamesadarich avatar May 03 '18 21:05 jamesadarich

Hey, good timing! I just sat down at my computer and saw you had replied. :-)

So, the number one place I've encountered it is when capturing errors that only might be thrown. In other places, like say an equality check, you can work around it by simply providing different values to toEqual using TestCase.

Here's an example from a test covering the allSatisfy operator in the plugin I was working on:

  @TestCase([1, 2, 3], (e: number) => e < 5, false)
  @TestCase([1, 2, 3, 6], (e: number) => e < 5, true)
  public allSatisfy_passesIffAll(
    array: Array<any>,
    predicate: (a: any) => boolean,
    throws: boolean
  ) {
    const fn = () => Assert(array).allSatisfy(predicate);
    Assert(fn).maybe(throws).throws(SpecError);
  }

Obviously, this could have been two methods--one for passes, one for fails--but this way reduces redundancy. With Alsatian's Expect, I think it's most helpful with the toThrow* and toHaveBeenCalled* family of methods.

cdibbs avatar May 03 '18 21:05 cdibbs

Hey @cdibbs, sorry about the late reply. Been very busy in my non-GitHub life recently so missed it, but I'm back! :)

If this is still a concern for you I'd probably from a readability perspective and a testing perspective split this into two separate tests as it's testing two distinct things. Usually when an error is thrown this is one unit of functionality and scenarios in which it doesn't is another.

  @TestCase([1, 2, 3], (e: number) => e < 4)  
  @TestCase([1, 2, 3], (e: number) => e < 5)
  @TestCase([1, 2, 3, 42], (e: number) => e < 43)
  public doesntThrowIfAllBelowTarget(
    array: Array<any>,
    predicate: (a: any) => boolean
  ) {
    const fn = () => Assert(array).allSatisfy(predicate);
    Expect(fn).not.toThrow(SpecError);
  }

  @TestCase([1, 2, 3, 4], (e: number) => e < 4)  
  @TestCase([6, 1, 2, 3], (e: number) => e < 5)
  @TestCase([1, 99, 3], (e: number) => e < 43)
  public doesThrowIfAnyNotBelowTarget(
    array: Array<any>,
    predicate: (a: any) => boolean
  ) {
    const fn = () => Assert(array).allSatisfy(predicate);
    Expect(fn).toThrow(SpecError);
  }

I think the above is generally how I'd do it, yes there's more lines going on but I think it's a bit clearer. What are your thoughts on the above approach?

jamesadarich avatar Jul 06 '18 20:07 jamesadarich

Hey @jamesadarich no worries! I totally understand. My seasons of open source work come and go, too. Lately, I don't even have a good excuse for why I haven't finished more quickly on some things I've been working on except to say that I've rediscovered Fallout 4 in VR! :-)

Anyway, yeah, the approach you show above is how I'd probably handle throws in some cases, too. At other times (and not necessarily related to throws), I've found good cause to make the occasional assertion conditional.

In any case, its definitely not a critical operator; I've just found it nice to have.

cdibbs avatar Jul 06 '18 21:07 cdibbs

Oh wow, is it totally worth it? I've been pondering about fallout vr myself.

I'm going to keep this issue open to investigate options on this one I think could certainly be potential :)

jamesadarich avatar Jul 06 '18 21:07 jamesadarich

Yeah, I got it on discount during the Steam sale. That and the bug fixes they released put it over the top, for me. If you enjoy the story, I'd say go for it.

cdibbs avatar Jul 06 '18 21:07 cdibbs