Skip to content

More reliable check_spec'ing for Cover-compiled modules. #54

Merged
merged 1 commit into from Mar 31, 2013

2 participants

@jhlywa
jhlywa commented Nov 27, 2012

check_spec indirectly used code:which/0 to load the BEAM of a targeted
module. If the module was compiled with {cover_enabled, true}, proper
failed to load the BEAM and resorted to recompiling the module with a
hardcode list of compilation options. If the targeted module depended
on any additional flags (such as include dirs), the compilation failed
and check_spec returned an error.

This fix uses code:get_object_code/1 (instead of code:which/1) to
retrieve the BEAM for a given module, sidestepping the need to
recompile any Cover-compiled modules.

@jhlywa jhlywa Cover-compiled modules are now more reliably check_spec'ed
check_spec indirectly used code:which/0 to load the BEAM of a targeted
module.   If the module was compiled with {cover_enabled, true}, proper
failed to load the BEAM and resorted to recompiling the module with a
hardcode list of compilation options.  If the targeted module depended
on any additional flags (such as include dirs), the compilation failed
and check_spec returned an error.

This fix uses code:get_object_code/1 (instead of code:which/1) to
retrieve the BEAM for a given module, sidestepping the need to
recompile any Cover-compiled modules.
b9db143
@kostis
Collaborator
kostis commented Nov 27, 2012

Thanks for this pull request. It can of course be merged as is but, since this is exactly the kind of code that may easily break by accident as there is nothing that exercises it and it's not so common that users have cover-compiled modules, we would appreciate if you can include a small test together with the patch.

The (unit) tests of PropEr are normally run with

make tests

at the top dir. Let us know.

@kostis
Collaborator
kostis commented Dec 6, 2012

Ping.

Are you working on an appropriate test for this, or should we merge this pull request as is?

@jhlywa
jhlywa commented Dec 6, 2012

Sorry for the delay. I've been working on a few check_spec tests and should have something for you soon.

@kostis
Collaborator
kostis commented Feb 17, 2013

Hi @jhlywa

It seems that you are even more swamped than we are... Can we expect the tests for this issue sometime soon?

Kostis

@kostis kostis was assigned Feb 28, 2013
@kostis kostis merged commit a5bd59a into manopapad:master Mar 31, 2013
@kostis
Collaborator
kostis commented Mar 31, 2013

I've merged the pull request, because I think it's an improvement, but I still do not like it that we have no tests for this. If you find some time, tests would be appreciated. Otherwise, this functionality runs the risk of being accidentally broken in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.