Can we make `containsAll` fail when the `expected` argument is an empty list?

[matthid]

In https://github.com/fsharp/FAKE/pull/2424 one of the problems was that the arguments are mixed and an error slipped through:

We could make it fail with a message that an empty list doesn't make sense here and ask if the arguments are potentially mixed up.

[haf]

We could warn with the expecto logger, but I'm not much for failing with exceptions for passing valid input into the expect function.

[matthid]

I know what you mean. My thinking is that when expected is an empty list the assert is basically a noop and the expected value should be static test data. So what is the scenario for this. I cannot think of a case where this is useful

[haf]

I can imagine testing a property of the co-domains of two functions, that for some input both return [], like: Expect.containsAll [] [] "equivalence"

[SteveGilham]

Given that this was an argument ordering problem at base -- and unit testing frameworks are divided over which of actual and expected comes first, rather than following a universal convention -- a better approach might be along the lines of offering an alternative set of APIs with a record like this as input

type Match<'a> =  // or some better name
  { 
      Actual : 'a
      Expected : 'a
  }

that feeds explicitly named values into the assertion.

[haf]

@SteveGilham Yes that might be a third option (we have both ordering variants, in open Expecto and open Expecto.Flip respectively); I think matchers generally could be a great way to encapsulate what has been asserted, but I would want more bang for the buck if I were to add that API, than your example API has.

I'm closing this since it's not that actionable for me. Ping me if you want to reopen, or even better, have a PR where you're fleshing out the API you would want (so we can discuss it)

[matthid]

@haf given your example of both lists being empty: What about only throwing if expected is empty and actual is not (which was the case reported above)

[haf]

I'd be happy to have it warn in that case, as long as I can override the warning to silence it with the config. Alternatively you can introduce a configuration flag that changes the assertion logic, as long as that flag is opt-in.

I absolutely see where you're coming from, but the reason I don't agree to it is that it breaks the symmetry one expects from mathematics; that any set contains the empty set https://www.math.drexel.edu/~tolya/emptyset.pdf

[matthid]

but the reason I don't agree to it is that it breaks the symmetry one expects from mathematics; that any set contains the empty set

Yes, I know it is "wrong" mathematically, but I still cannot find any practical relevance for the situation I described above (IMHO the benefit is higher than the cost). Additionally, we are talking about a unit testing API here, not on a mathematical operation.

But stepping back a bit. The issue was only to show a potential pitfall and start the discussion around a potential solution. I'm pretty sure a warning wouldn't have helped us so we should leave it as-is. :)

[haf]

Yes, I know it is "wrong" mathematically, but I still cannot find any practical relevance for the situation I described above (IMHO the benefit is higher than the cost).

The thing is that I tend to encode mathematical properties like this with Expecto.FsCheck when I test my code, so it'll lead to breakages for me ;)