-
Notifications
You must be signed in to change notification settings - Fork 141
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
Do not depend on directory #321
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about defining our own removeFile
instead of using c_unlink
directly in multiple places?! I think that might be more readable and would help keep our tests accessible for future contributors.
@sjakobi good point, done. I also switched from hardcoded filenames to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I guess we could have a shared TestUtils
module, so we don't need multiple removeFile
definitions. I don't mind too much though.
These are all different test components, so it is a bit cumbersome to share a hypothetical |
Wouldn't it be as easy as listing the
We've considered doing this in |
It will work, but IMHO too much churn for a single helper function.
Could probably be the case for more diverse or extremely large test suites. On the other hand, it saves linking time and generates a single, aggregated test report. |
* Use c_unlink instead of removeFile * Freshen up LazyHClose tests * Avoid directory package in builder tests * Do not depend on directory package * Review suggestions
This is a necessary prerequisite for #316: if we want to move tests to the
bytestring
package itself, test components should avoid packages, which depend onbytestring
. Except replacingtest-framework
with something having smaller dependency footprint (there is a good progress on this), the only remaining offender isdirectory
, which depends onbytestring
transitively viaunix
andWin32
packages.This PR unties
bytestring-tests
fromdirectory
:removeFile
can be replaced with a POSIXc_unlink
.Not sure what exactly
renameFile
was meant to test inLazyHClose.hs
(this file was not run for ages, and only recently salvaged and integrated in the test suite). If the purpose was to ensure, that a file is properly closed, opening it for write should be enough (and better).I had to sacrifice
getTemporaryDirectory
call in builder tests. Given that all other test components do not bother with it and create files in the current folder, it seems to be an acceptable tradeoff.