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

Argument error when reporting results #1

Closed
keathley opened this issue Jul 26, 2019 · 9 comments
Closed

Argument error when reporting results #1

keathley opened this issue Jul 26, 2019 · 9 comments

Comments

@keathley
Copy link

keathley commented Jul 26, 2019

Hey @dnlserrano! I tried this out on my norm library and I got an argument error from the reporter. It looks like its trying to convert a string to an existing atom and it can't find the atom or something similar. Here's the full output:

keathley in ~/D/norm on improve-docs
λ env MIX_ENV=test mix exavier.test
Compiling 11 files (.ex)
Generated norm app

08:47:04.990 [error] GenServer :exavier_reporter terminating
** (ArgumentError) argument error
    :erlang.binary_to_existing_atom("Elixir.Norm.Selection", :utf8)
    (exavier) lib/exavier.ex:75: Exavier.string_to_elixir_module/1
    (exavier) lib/exavier/reporter.ex:34: Exavier.Reporter.handle_cast/2
    (stdlib) gen_server.erl:637: :gen_server.try_dispatch/4
    (stdlib) gen_server.erl:711: :gen_server.handle_msg/6
    (stdlib) proc_lib.erl:249: :proc_lib.init_p_do_apply/3
Last message: {:"$gen_cast", {:test_finished, %ExUnit.Test{case: Norm.SelectionTest, logs: "", module: Norm.SelectionTest, name: :"test generation can generate values", state: nil, tags: %{async: true, case: Norm.SelectionTest, describe: "generation", describe_line: 58, file: "/Users/keathley/Development/norm/test/norm/selection_test.exs", line: 59, module: Norm.SelectionTest, registered: %{}, test: :"test generation can generate values", test_type: :test}, time: 190}}}
State: %Exavier.Reporter{all_failures: [], mutated_modules: %{}}
** (EXIT from #PID<0.92.0>) an exception was raised:
    ** (ArgumentError) argument error
        :erlang.binary_to_existing_atom("Elixir.Norm.Selection", :utf8)
        (exavier) lib/exavier.ex:75: Exavier.string_to_elixir_module/1
        (exavier) lib/exavier/reporter.ex:34: Exavier.Reporter.handle_cast/2
        (stdlib) gen_server.erl:637: :gen_server.try_dispatch/4
        (stdlib) gen_server.erl:711: :gen_server.handle_msg/6
        (stdlib) proc_lib.erl:249: :proc_lib.init_p_do_apply/3

I'm running on elixir 1.9.1 and OTP 22 if that helps. I've also tested this with the most recent version of exavier on hex and with the master branch. The failure is non-deterministic and I suspect that just reporting whatever test it receives first since they're running async. I suspect that it would be safe to use to_atom here since this will really only be run in test mode and we don't really have to worry about DOSing the atom table. But I figured I'd leave that up to you.

@dnlserrano
Copy link
Owner

Thanks @keathley I'll try and take a look today/tomorrow.

@jdugarte
Copy link

I'm getting a similar error:

$ mix exavier.test
==> exavier
Compiling 15 files (.ex)

Generated exavier app
==> workforce

Generated workforce app
** (EXIT from #PID<0.91.0>) an exception was raised:
    ** (ArgumentError) argument error
        :erlang.binary_to_existing_atom("Elixir.Workforce.PersistUser", :utf8)
        (exavier) lib/exavier.ex:75: Exavier.string_to_elixir_module/1
        (exavier) lib/exavier/reporter.ex:34: Exavier.Reporter.handle_cast/2
        (stdlib) gen_server.erl:637: :gen_server.try_dispatch/4
        (stdlib) gen_server.erl:711: :gen_server.handle_msg/6
        (stdlib) proc_lib.erl:249: :proc_lib.init_p_do_apply/3

@dnlserrano
Copy link
Owner

So sorry for the lousy experience guys, looking at it rn.

@keathley
Copy link
Author

@dnlserrano No worries :). These things happen.

@dnlserrano
Copy link
Owner

dnlserrano commented Jul 26, 2019

I just had a moment to look at this, I'll try and post more updates tomorrow, but gist of it is:

  1. I should use String.to_atom instead of String.to_existing_atom. I suspect it was working for me before because I was running exavier against something I had inside the exavier project itself (which would be compiled as part of the normal mix compilation of the exavier project alongside it – hence making use of the atom of that module).

I suspect that it would be safe to use to_atom here since this will really only be run in test mode and we don't really have to worry about DOSing the atom table.

Agreed.

  1. After I fixed that and ran it again on norm, I get an arithmetic error because apparently it isn't mutating anything and I end up dividing by 0 in the reporter. This is rather embarassing. The bug for division by 0 is an easy one to fix, but I have to try and understand why it even happens in the first place. May be 1 of 3:

a) existing mutators are not enough to change the norm code base (I recall we only have 5 PoC mutators for now);

b) cover might not be picking up test covered lines it should be picking up;

c) I might have some serious bug, or this might all be worthless. 😩 I hope not. 😭 Is this what open-source life looks like? 💔 Jk.

I have to get out of the PC for now. I'll try to get back at it tomorrow morning. Thanks again for trying it out guys, it means a lot! And sorry again for the trouble you ran into. Don't give up just yet! Pretty please. 🤞

@dnlserrano
Copy link
Owner

Ah and btw, I also had to tweak the file tree structure for your test/ folder because right now we have to have a perfect mapping for exavier to discover files (e.g., test/norm/spec/selection_test.exs must map to exactly lib/norm/spec/selection.ex.

Maybe there is a better way of doing this. 🙈

dnlserrano added a commit that referenced this issue Jul 27, 2019
dnlserrano added a commit that referenced this issue Jul 27, 2019
Noticed after running this on `norm` by Chris Keathley.

I guess I was too high on ignoring warnings due to compiling only
changed stuff while hacking this in the last commits. My gawd.

Relates to issue #1
dnlserrano added a commit that referenced this issue Jul 27, 2019
* Also, add pattern match safe-guard to ensure module was compiled correctly,
without errors.

Relates to issue #1
dnlserrano added a commit that referenced this issue Jul 27, 2019
If we weren't able to do any mutations, we should report back 100%
mutation coverage. Or should we? 😈

Relates to issue #1
@dnlserrano
Copy link
Owner

I've pushed another release, which should potentially fix some of the reported issues.

I think right now option b) holds strong, which makes me think maybe I shouldn't be running code coverage at all before trying to mutate the code. But that's part a bigger discussion, I think (on the "To be done" section of the README).

TL;DR I think is in norm Erlang's :cover can't see some of the lines of code that were run and that causes me to not have interest in mutating them.

You can try it out yourself with these changes @keathley. I default to reporting 100% mutation coverage when I mutate nothing, which seems like a good default for now (since ATM we know the lib is at a very young state to be able to raise red flags in such a scenario).

norm is raising me a lot of warning when running exavier, which I didn't have time to look at just yet, but you might have an idea of what's happening, maybe @keathley?

What do you guys think? Feedback is of course very much appreciated.

Thanks again so much for giving this a go! ❤️

@dnlserrano
Copy link
Owner

@keathley @jdugarte ping

Did you guys get the chance to try it again? I have pushed some extra changes. Particularly, with v0.1.4 you get two new config options.

I also got mix exavier.test to run on exavier's test pipeline in Travis CI. 😊 Dog fooding all the way!

@dnlserrano
Copy link
Owner

Closing this.

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