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

Add conditional negation operator. #461

Open
cdibbs opened this issue Apr 30, 2018 · 6 comments
Open

Add conditional negation operator. #461

cdibbs opened this issue Apr 30, 2018 · 6 comments

Comments

@cdibbs
Copy link

cdibbs commented Apr 30, 2018

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();
@jamesadarich
Copy link
Member

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

@cdibbs
Copy link
Author

cdibbs commented May 3, 2018

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.

@jamesadarich
Copy link
Member

jamesadarich commented Jul 6, 2018

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?

@cdibbs
Copy link
Author

cdibbs commented Jul 6, 2018

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.

@jamesadarich
Copy link
Member

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

@cdibbs
Copy link
Author

cdibbs commented Jul 6, 2018

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.

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