propcheck icon indicating copy to clipboard operation
propcheck copied to clipboard

No function clause when trying to generate `pos_integer()`

Open adkron opened this issue 2 years ago • 12 comments

Version

Propcheck v 1.4.1

Example Code

let count <- pos_integer() do
  (1..count) |> Enum.to_list()
end

Error

     ** (FunctionClauseError) no function clause matching in :proper_arith.rand_non_neg_float/1

     The following arguments were given to :proper_arith.rand_non_neg_float/1:
     
         # 1
         :undefined
     
     stacktrace:
       (proper 1.4.0) /Users/amosking/workspace/my/deps/proper/src/proper_arith.erl:303: :proper_arith.rand_non_neg_float/1
       (proper 1.4.0) /Users/amosking/workspace/my/deps/proper/src/proper_arith.erl:258: :proper_arith.rand_non_neg_int/1
       (proper 1.4.0) /Users/amosking/workspace/my/deps/proper/src/proper_gen.erl:368: :proper_gen.integer_gen/3
       (proper 1.4.0) /Users/amosking/workspace/my/deps/proper/src/proper_gen.erl:198: :proper_gen.generate/3
       (proper 1.4.0) /Users/amosking/workspace/my/deps/proper/src/proper_gen.erl:136: :proper_gen.generate/1
       (proper 1.4.0) /Users/amosking/workspace/my/deps/proper/src/proper_gen.erl:557: :proper_gen."-fixed_list_gen/1-lc$^1/1-0-"/1
       (proper 1.4.0) /Users/amosking/workspace/my/deps/proper/src/proper_gen.erl:557: :proper_gen."-fixed_list_gen/1-lc$^1/1-0-"/1
       (proper 1.4.0) /Users/amosking/workspace/my/deps/proper/src/proper_gen.erl:524: :proper_gen.tuple_gen/1
       (proper 1.4.0) /Users/amosking/workspace/my/deps/proper/src/proper_gen.erl:198: :proper_gen.generate/3
       (proper 1.4.0) /Users/amosking/workspace/my/deps/proper/src/proper_gen.erl:136: :proper_gen.generate/1
       (proper 1.4.0) /Users/amosking/workspace/my/deps/proper/src/proper_gen.erl:186: :proper_gen.generate/3
       (proper 1.4.0) /Users/amosking/workspace/my/deps/proper/src/proper_gen.erl:136: :proper_gen.generate/1
       (proper 1.4.0) /Users/amosking/workspace/my/deps/proper/src/proper_gen.erl:557: :proper_gen."-fixed_list_gen/1-lc$^1/1-0-"/1
       (proper 1.4.0) /Users/amosking/workspace/my/deps/proper/src/proper_gen.erl:524: :proper_gen.tuple_gen/1
       (proper 1.4.0) /Users/amosking/workspace/my/deps/proper/src/proper_gen.erl:198: :proper_gen.generate/3
       (proper 1.4.0) /Users/amosking/workspace/my/deps/proper/src/proper_gen.erl:136: :proper_gen.generate/1
       (proper 1.4.0) /Users/amosking/workspace/my/deps/proper/src/proper_gen.erl:186: :proper_gen.generate/3
       (proper 1.4.0) /Users/amosking/workspace/my/deps/proper/src/proper_gen.erl:136: :proper_gen.generate/1
       (proper 1.4.0) /Users/amosking/workspace/my/deps/proper/src/proper_gen.erl:557: :proper_gen."-fixed_list_gen/1-lc$^1/1-0-"/1
       (proper 1.4.0) /Users/amosking/workspace/my/deps/proper/src/proper_gen.erl:557: :proper_gen."-fixed_list_gen/1-lc$^1/1-0-"/1

adkron avatar Nov 12 '21 20:11 adkron

@adkron, I guess you are omitting something. To put your example in a full property I did this

property "let with pos_integer fails", [:verbose] do
    our_list = let count <- pos_integer() do
      (1..count) |> Enum.to_list()
    end

    forall l <- our_list do
      (length(l) >= 1)
      |> measure("PosInt List length", length l)
      |> collect(length l)
    end
  end

Works without any problems (and I would be very surprised if you really find a bug in the pos_integer() generator, which exists for many years in both, PropEr and PropCheck, without any changes).

alfert avatar Nov 14 '21 10:11 alfert

       (proper 1.4.0) /Users/amosking/workspace/my/deps/proper/src/proper_arith.erl:303: :proper_arith.rand_non_neg_float/1

For this to be called with undefined, the Size parameter of integer_gen/3 needs to be set to undefined:

%% I picked the clause for Line 368 as in the issue descriptions stacktrace.
integer_gen(Size, Low, inf) ->
    Low + proper_arith:rand_non_neg_int(Size);

@adkron did you maybe use sized/2 or something similar to configure the size?

evnu avatar Nov 15 '21 08:11 evnu

I did not try to configure size.

I was able to narrow it down, and I don't know if this should work or not.

I am using Propcheck.produce/2 inside of a let. I had to dig into a few function calls to find that out.

adkron avatar Nov 15 '21 17:11 adkron

Ouch, that feels like too many levels of abstractions :-) if I understand your statement correctly. The tuple return value of produce/2 can do all kinds of surprising things ...

alfert avatar Nov 15 '21 22:11 alfert

I can produce a similar crash with this example:

defmodule BlablaTest do
  use ExUnit.Case
  use PropCheck

  property "boom" do
    gen = let x <- pos_integer() do
      {:ok, some_other} = produce(pos_integer())
      {x, some_other}
    end

    forall {x, y} <- gen do
      x + y >= 0
    end
  end
end

Stacktrace:

  1) property boom (BlablaTest)
     test/blabla_test.exs:5
     ** (FunctionClauseError) no function clause matching in :proper_arith.rand_non_neg_float/1

     The following arguments were given to :proper_arith.rand_non_neg_float/1:

         # 1
         :undefined

     code: property "boom" do
     stacktrace:
       (proper 1.4.0) /tmp/blabla/deps/proper/src/proper_arith.erl:303: :proper_arith.rand_non_neg_float/1
       (proper 1.4.0) /tmp/blabla/deps/proper/src/proper_arith.erl:258: :proper_arith.rand_non_neg_int/1
       (proper 1.4.0) /tmp/blabla/deps/proper/src/proper_gen.erl:368: :proper_gen.integer_gen/3
       (proper 1.4.0) /tmp/blabla/deps/proper/src/proper_gen.erl:198: :proper_gen.generate/3
       (proper 1.4.0) /tmp/blabla/deps/proper/src/proper_gen.erl:136: :proper_gen.generate/1
       (proper 1.4.0) /tmp/blabla/deps/proper/src/proper_gen.erl:186: :proper_gen.generate/3
       (proper 1.4.0) /tmp/blabla/deps/proper/src/proper_gen.erl:136: :proper_gen.generate/1
       (proper 1.4.0) /tmp/blabla/deps/proper/src/proper_gen.erl:123: :proper_gen.safe_generate/1
       (proper 1.4.0) /tmp/blabla/deps/proper/src/proper.erl:1690: :proper.run/3
       (proper 1.4.0) /tmp/blabla/deps/proper/src/proper.erl:1518: :proper.perform/7
       (proper 1.4.0) /tmp/blabla/deps/proper/src/proper.erl:1257: :proper.inner_test/2
       (proper 1.4.0) /tmp/blabla/deps/proper/src/proper.erl:1239: :proper.test/2
       (propcheck 1.4.1) lib/properties.ex:183: PropCheck.Properties.execute_property/4
       test/blabla_test.exs:5: (test)

@adkron Do you need to use produce in a let? Note that you can also nest let if this is about dependencies between generated values.

evnu avatar Nov 16 '21 13:11 evnu

I don't need to use the 'let'. I worked around it. We use produce in some tests that aren't properties to generate some random test data.

The bigger issue is that it isn't clear that you can't use produce inside the 'let'. The error message isn't helpful either. It leads to wrong assumptions.

Great example. Here is the crazy part on my system. When I was trying this if the call to 'pos_integer' on the 'let' line is changed to something like 'oneof([1, 2, 3])' I wouldn't get an error. That made it seem like the 'let' line was the issue.

I think that this will bite people and they won't know the issue. If there is a way that we can save them some time if we can't make it work. I'm happy to help. I've been digging in and trying to figure out where to solve this.

I'm not at a computer so I can't test this at the moment but I will later.

adkron avatar Nov 16 '21 13:11 adkron

      property "boom" do
        gen = let x <- oneof([1,2,3]) do
          {:ok, some_other} = produce(pos_integer())
          {x, some_other}
        end

        forall {x, y} <- gen do
          x + y >= 0
        end
      end

It is verified that this passes. The issue only occurs with there is an integer generator inside an integer generator.

adkron avatar Nov 16 '21 17:11 adkron

I can support this, also floats do not work. Binaries are not problem.

Since PropCheck delegates to the PropEr for the implementation of of the generators, I assume that we experience the same strange things also in Erlang. On the other hand, the use of produce in a generator is strange by itself, since you would normally bind the generator to a new variable (either in let or in forall). From that perspective produce is really meant for interactive exploration of generators and outside of forall and let constructs. Is that something we should add to the documentation? We can of course mention, that in some cases unpredictable (or confusing) behaviours appear if somebody mixes this two operating modes.

alfert avatar Nov 16 '21 20:11 alfert

Using produce should work even though it isn't the ideal path, or we need the error to say something about the issue. We can also update the docs. I'm willing to help implement any solutions. I'm just not sure where to start.

I'm throwing out an idea here. Should produce be in another module that is only for playing in a console?

adkron avatar Nov 17 '21 15:11 adkron

Should produce be in another module that is only for playing in a console?

That's a good idea! Detecting "improper" use of produce is hard, but we can at least make it obvious what's its intended use is for.

If we do that, we need to declare PropCheck.produce/1 deprecated until we release a new major version of PropCheck, as this is a breaking change.

evnu avatar Nov 18 '21 07:11 evnu

I like the idea of putting some functions in a "interactive" or "console" package. produce() belongs there, sample_shrink() also. quickcheck() is also a candidate, as are the reporting functions like collect() or measure().

The notion would be that these functions are usually used in an interactive setting to try out things, but most often not used in test suites. We would need than an explicit warning of the strange behaviour of produce as Amos observed.

alfert avatar Nov 23 '21 09:11 alfert

I use collect and measure a lot within my tests to get some statistics on what is happening. Maybe I should be doing that from the console instead.

adkron avatar Nov 23 '21 20:11 adkron