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

With a bogus test instantiator, meck fails with {error, enoent} in meck_cover:read_cover_file/1 #114

Closed
rlipscombe opened this issue Nov 28, 2013 · 22 comments
Labels

Comments

@rlipscombe
Copy link
Contributor

If I have a test instantiator looking like this:

all_test_() ->
  {foreach, fun setup/0, fun cleanup/1, [ fun test_thing/1 ]}.
setup() -> meck:new(foo, [passthrough]).
cleanup(_) -> meck:unload(foo).
test_thing(_) ->
  ?assert(true).

...eunit reports "result from instantiator foo_tests:'-all_test_/0-fun-2-'/1 is not a test", which is fair enough, but then meck fails as follows:

14:45:36.177 [error] gen_server foo_meck terminated with reason: no match of right hand value {error,enoent} in meck_cover:read_cover_file/1
14:45:36.177 [error] CRASH REPORT Process foo_meck with 0 neighbours exited with reason: no match of right hand value {error,enoent} in meck_cover:read_cover_file/1 in gen_server:terminate/6

(I've ellided the line numbers, because this I've been hacking on it and don't want to confuse anyone).

Further investigation seems to show meck_cover:read_cover_file being called with a relative path "foo_meck_original.coverdata", but with file:get_cwd() returning "/path/to/my/apps/foo", rather than "/path/to/my/apps/foo/.eunit".

If I hack on meck_cover:read_cover_file to make it also look in the .eunit folder, it works -- at least until it barfs in meck_proc:restore_original instead.

This is using rebar eunit, by the way. It looks like either eunit or (more likely) rebar are changing cwd underneath meck's feet.

@eproxus
Copy link
Owner

eproxus commented Dec 2, 2013

So, the question is, does it do this between meck:new/2 and meck:unload/1? If so, I would consider that a bug in Rebar, but it sounds unlikely that they would do something like this...

@rlipscombe
Copy link
Contributor Author

Good question. My Erlang-fu isn't strong enough to figure that out yet. I figured that -- maybe -- rebar is calling eunit:test(), which is failing, but then there's a race between rebar changing the working directory, and meck's gen_server being torn down (it's usually torn down in unload, right, but in this instance, it's doing it because of the error?).

@eproxus
Copy link
Owner

eproxus commented Dec 9, 2013

Sorry for the late reply.

I guess the error does not appear if you fix the test (i.e. remove the "is not a test" error)?

@rlipscombe
Copy link
Contributor Author

Correct. When it is a test, whether it passes or fails, then it cleans up properly.

It looks like a race condition in tear down, but only in this case (or maybe in similar cases).

(tbh, I'm not that bothered about it now, since I've figured it out, and Google returns this issue as the top result for the error message, so if it's a PITA to fix, don't worry about it, and thanks for all that you've done with meck).

@eproxus
Copy link
Owner

eproxus commented Dec 10, 2013

I think the correct way to pass an argument to a test is something like this (taken from meck_test.erl):

{foreach, fun setup/0, fun teardown/1,
 [{with, [T]} || T <- [fun ?MODULE:one/1,
                       fun ?MODULE:two/1]]}

It's kind off ugly, but the only way to tell EUnit to pass an argument to each test case. My theory is that the setup function is run before EUnit examines your test case fun and thus fails, bringing Meck down with it. This is "expected" behaviour. The problem is more or less the design of EUnit in that it doesn't verify the test specification before running any of the functions (which means cleanup is also broken etc.).

@eproxus
Copy link
Owner

eproxus commented Feb 3, 2014

Closing because of inactivity. Please re-open if it is still an issue.

@eproxus eproxus closed this as completed Feb 3, 2014
@qrilka
Copy link

qrilka commented Feb 18, 2014

I get the same error with the following test (with no "is not a test" error):

my_test_() ->
{
  setup,
  fun() ->
    meck:new(boo)
  end,
  fun(_) ->
    meck:unload(boo)
  end,
  [fun() -> ?assert(true) end]
}
.

Installed meck today, any hints?

@eproxus
Copy link
Owner

eproxus commented Feb 18, 2014

Cannot really reproduce this. A freshly created rebar app with nothing but the exact same test case you posted and Meck as a dependency works for me.

$ cat rebar.config
{deps, [
    {meck, "", {git, "https://github.com/eproxus/meck.git"}}
]}.
$ cat test/test_test.erl
-module(test_test).

-include_lib("eunit/include/eunit.hrl").

my_test() ->
    {
        setup,
        fun() ->
            meck:new(boo)
        end,
        fun(_) ->
            meck:unload(boo)
        end,
        [fun() -> ?assert(true) end]
    }.
$ rebar eunit skip_deps=true
==> test (eunit)
Compiled test/test_test.erl
  Test passed.

@qrilka
Copy link

qrilka commented Feb 18, 2014

It seems to be something coming from other tests - I could not reproduce it in a separate module either. Need to investigate.

@cwmaguire
Copy link

I came across this today. I had three modules: module 'a' had code and tests, module 'b' had code, module b_tests was tests for 'b' (e.g. 'a', 'b', 'b_tests'). Module b was mecking out module a. Once I removed the tests from module a and put them in a_tests, the problem went away.

@eproxus
Copy link
Owner

eproxus commented Mar 13, 2014

@cwmaguire Did you mock module a in both tests and run them in parallel? Meck is creating temporary files with cover info when running tests, and for some reason it seems that these are "disappearing" between or during tests somehow.

@cwmaguire
Copy link

No, I didn't mock a in both tests (not in both module a + module b_tests or in multiple, parallel tests in module b_tests) or explicitly run any tests in parallel. I did find that file:get_cwd() in read_cover_file(File) was returning not the .eunit directory but the directory above it, and File was relative ("a_meck_original.coverdata"); also, both the original a.coverdata and the temporary a_meck_original.coverdata existed after the tests failed.

@eproxus
Copy link
Owner

eproxus commented Mar 14, 2014

Hmm, the assumption that meck:new and meck:unload are run in the same directory have held so far. :-) I guess it is not the case here?

@qrilka
Copy link

qrilka commented Mar 17, 2014

It looks like I have found a way to reproduce that enoent:
https://github.com/qrilka/meck_test
and gist of running eunit on my machine - https://gist.github.com/qrilka/9595222
So it appears that the problems may be in test code (like undefined_function in this example) but a problem in export_original_cover and error report from it make real problem quite hard to find.
I don't have a soluition for this issue with export_original_cover though (at least yet).
@eproxus maybe this issue should be reopened or I need to file a new issue?

@robertoaloi
Copy link

Having a similar issue. It looks like some weird interaction between rebar_eunit and meck_cover goes on. This only happens if cover_enabled is set to true in rebar.config. After a quick investigation, it looks like the following line (extracted from a rebar_eunit.erl belonging to an oldish version of rebar based on 2.1.0-pre) is somehow involved:

{ok, _} = cover:analyze_to_file(M, cover_file(M), [html])

Commenting out the above line does not cause meck to crash. It looks like the latest version of rebar does things quite differently, so I will try to investigate more.

@eproxus
Copy link
Owner

eproxus commented Apr 14, 2014

@robertoaloi So, ideally, Meck should only mess with cover data on mocking and unloading modules (which should happen inside tests) and Rebar (hopefully) only after tests have been run. If you've found a bug here, I would be interested to hear about it. 😄

@ferrouswheel
Copy link

I've run into this a couple of times.

Each time has been due to using an ?assert* in a test generator instead of the correct ?_assert* variant.

So I don't think meck is to blame, but it'd be nice if there was a more obvious error message!

@eproxus
Copy link
Owner

eproxus commented Jun 12, 2014

@qrilka @robertoaloi So, after some more investigation I think I figured out what happens. You are mocking a, which doesn't have a function b/0 and Meck throws the error {undefined_function,{a,b,0}} because the option non_strict is not used (i.e. you must only mock functions which exist in the module in the first place). This happens during setup. Then EUnit gets the exception (because it is not catched in the setup function) and crashes the test process, which in turn crashes the Meck process (because it is linked to the test process). That results in the error report seen later. As an icing on the cake, Meck tries to restore the cover data which seems to not exist (it should have been created during meck:new(...)).

And this is really the strange part. If I look into the .eunit folder Rebar produces there is a a.coverdata file in there. Perhaps the directory is change by Rebar or something during the setup/crash handling?

@qrilka
Copy link

qrilka commented Jun 15, 2014

@eproxus the code is on github, I don't do anything special myself and I didn't dig deeper into this issue so I don't have any other details on this

@complex64
Copy link

+1

eproxus added a commit that referenced this issue Jun 9, 2015
Turns out that we can't rely on the current working directory always
staying the same. The real world scenario is when Meck is cleaning up
after the last failed test case and Rebar changes directory from the
.eunit folder to the project root. Meck now stores absolute paths to
cover backup files.
@eproxus
Copy link
Owner

eproxus commented Jun 9, 2015

This issue should be fixed now. The solution was pretty simple, just save the absolute path of cover files instead of relying on the current directory always staying the same.

@complex64
Copy link

Awesome stuff, thanks Adam!

@eproxus eproxus added the bug label Jun 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants