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

Please add a prefix to test-helper.el to avoid conflicts with 68 other packages #708

Closed
tarsius opened this issue Feb 11, 2017 · 6 comments

Comments

@tarsius
Copy link

tarsius commented Feb 11, 2017

There exist at least 69 packages that contain a file named test-helper.el that also provides the feature test-helper.

This leads to issues for users who have at least two of these packages installed. It is unlikely that such a user would be able to run the tests of all of those packages. If the primary test file of one of those packages does (require 'test-helper), then it is undefined which of the various test-helper.el files gets loaded. Which it is, depends on the order of the load-path.

To avoid this conflicts, you should rename your test-helper.el to <your-package>-test-helper.el and adjust the feature and symbol prefixes accordingly.

Also don't forget to update the require form in your primary test file and/or update references to the library/feature elsewhere. Also, if your primary test file is named something like test.el, then please consider renaming that too (same for any other utility elisp files your repositoroy may contain).

Thanks!

PS: This issue is a bit generic because I had to open 69 issues.

@Fuco1
Copy link
Owner

Fuco1 commented Feb 11, 2017

And this is why people should use tests from a clean environment :D

Anyway, I can do this, it should be fairly simple change.

@tarsius
Copy link
Author

tarsius commented Feb 11, 2017

Well, yes.

Yes, please.

;-)

@tarsius
Copy link
Author

tarsius commented Feb 11, 2017

I've been informed, that ert-runner automatically loads the test-helper.el file. So you should not rename that file. Instead please see this issue, where I suggest that the (provide 'test-helper) should be dropped instead. Please accept my apologize for opening these issues prematurely.

@tarsius
Copy link
Author

tarsius commented Feb 20, 2017

The maintainer of ert-runner agrees that it would be best to remove (provide 'test-runner), and I would like to now encourage you to make that change. For some opinions on the matter please see rejeep/ert-runner.el#38.

Thanks for looking into this and sorry again for jumping the gun a bit.

@Fuco1 Fuco1 closed this as completed in 290ce9f Feb 20, 2017
@Fuco1
Copy link
Owner

Fuco1 commented Feb 20, 2017

Feel free to reopen if something more is needed :)

@tarsius
Copy link
Author

tarsius commented Feb 20, 2017

Thanks!

jojojames pushed a commit to jojojames/smartparens that referenced this issue Mar 31, 2017
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

2 participants