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

No function clause when trying to generate pos_integer() #211

Open
adkron opened this issue Nov 12, 2021 · 12 comments
Open

No function clause when trying to generate pos_integer() #211

adkron opened this issue Nov 12, 2021 · 12 comments

Comments

@adkron
Copy link
Contributor

adkron commented Nov 12, 2021

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
@alfert
Copy link
Owner

alfert commented Nov 14, 2021

@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).

@evnu
Copy link
Collaborator

evnu commented Nov 15, 2021

       (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?

@adkron
Copy link
Contributor Author

adkron commented Nov 15, 2021

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.

@alfert
Copy link
Owner

alfert commented Nov 15, 2021

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 ...

@evnu
Copy link
Collaborator

evnu commented Nov 16, 2021

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.

@adkron
Copy link
Contributor Author

adkron commented Nov 16, 2021

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
Copy link
Contributor Author

adkron commented Nov 16, 2021

      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.

@alfert
Copy link
Owner

alfert commented Nov 16, 2021

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.

@adkron
Copy link
Contributor Author

adkron commented Nov 17, 2021

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?

@evnu
Copy link
Collaborator

evnu commented Nov 18, 2021

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.

@alfert
Copy link
Owner

alfert commented Nov 23, 2021

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.

@adkron
Copy link
Contributor Author

adkron commented Nov 23, 2021

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.

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

3 participants