parroty / excheck

Property-based testing library for Elixir (QuickCheck style).
MIT License
316 stars 26 forks source link

Fix formatter crash in elixir 1.3.0 #25

Closed smpoulsen closed 8 years ago

smpoulsen commented 8 years ago

The formatter was crashing with a bad arithmetic error. This fixes the failure by updating the value and wrapping it back in a map before updating the config.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-10.3%) to 62.353% when pulling 18e7b9933e9e9acf3e4a852d7fb239d0d4df50c7 on tpoulsen:fix-failing-tests into cbc8f1e9d2c865dab048dd43a010dedda7a4a169 on parroty:master.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-10.3%) to 62.353% when pulling 18e7b9933e9e9acf3e4a852d7fb239d0d4df50c7 on tpoulsen:fix-failing-tests into cbc8f1e9d2c865dab048dd43a010dedda7a4a169 on parroty:master.

parroty commented 8 years ago

Thanks! I could confirm the failure and it's fixed with the commit

dantswain commented 8 years ago

@parroty @tpoulsen This is causing a crash of the formatter for me in Elixir 1.2.3

I get this:

16:28:15.013 [error] GenEvent handler ExCheck.Formatter installed in #PID<0.419.0> terminating
** (ArgumentError) argument error
    :erlang.apply(34, :test, [])
    (excheck) ExCheck.Formatter.handle_event/2
    (stdlib) gen_event.erl:538: :gen_event.server_update/4
    (stdlib) gen_event.erl:520: :gen_event.server_notify/4
    (stdlib) gen_event.erl:261: :gen_event.handle_msg/5
    (stdlib) proc_lib.erl:240: :proc_lib.init_p_do_apply/3
Last message: {:suite_finished, 26474, 159292}
State: %{colors: [enabled: true], failures_counter: 0, invalids_counter: 0, seed: 977788, skipped_counter: 0, tests_counter: 34, trace: true, width: 139}

The config is a map and config.tests_counter is a bare value, so calling config.tests_counter.test fails as :erlang.apply(34, :test, []) (in my particular case the value is 34). I've confirmed I don't get this error with ExCheck 0.3.3.

luc-tielen commented 8 years ago

A quick fix would be to update Elixir version to be >= 1.3.1 starting from ExCheck 0.4, would do this myself right now, but I get other errors when compiling dependencies (triq, random module is deprecated in erlang 19) so I can't verify myself..

smpoulsen commented 8 years ago

@dantswain Yeah, this isn't backwards compatible with Elixir < 1.3. The ExUnit formatter was changed here: https://github.com/elixir-lang/elixir/commit/c4d6be64581b9587dafe4ea5aef346e1684461da#diff-0ed5c47cc55f463c3af34af96d1fef62L18, requiring the map instead of bare value. https://github.com/parroty/excheck/pull/26 bumps the required version number for 0.4.0, but otherwise I'd go with @Primordus's suggestion.

luc-tielen commented 8 years ago

Was just thinking about this.. isn't it possible to add 2 clauses to the function: 1 for a map; 1 for the old version? Supports both versions and does not force people to update their Elixir version.. :smile: If I have time tonight, I'll try this myself, but otherwise feel free to go ahead and implement this.

smpoulsen commented 8 years ago

Ah, good call! Perhaps forced updates aren't the way to go after all 😉 I've got a bit of time this morning, so I'll have a go it.

smpoulsen commented 8 years ago

Submitted #27 that should handle Elixir versions pre 1.3 again. Successfully tested locally against 1.2.3, 1.2.6, and 1.3.1.

parroty commented 8 years ago

Thanks for the update. Merged and pushed as v0.4.1