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

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

Merged
merged 1 commit into from
Mar 31, 2013

Conversation

jhlywa
Copy link
Contributor

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

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

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

@ghost ghost assigned kostis Feb 28, 2013
kostis added a commit that referenced this pull request Mar 31, 2013
More reliable check_spec'ing for Cover-compiled modules.
@kostis kostis merged commit a5bd59a into proper-testing:master Mar 31, 2013
@kostis
Copy link
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants