cabal test fails the second time

[gelisam]

Because the first pass creates fakedir, so for the second pass, it's no longer a non-existing folder.

[gelisam]

Ever since I fixed #87, it fails the first time too :)

It's because #87 now creates the context directory without emitting a warning.

[melrief]

I don't see how: checkContextDir must emit the warning if the directory doesn't exist. Maybe we should remove any directory called fakedir before starting the test else the test will already find the directory? Or use a temporary filename as fake directory so that it cannot exist?

[gelisam]

I don't see how: checkContextDir must emit the warning if the directory doesn't exist.

Because I removed the call to checkContextDir in da5b015. I'll add it somewhere else.

Maybe we should remove any directory called fakedir before starting the test else the test will already find the directory?

I was planning to remove it after the test instead of before.

[gelisam]

Branch issue-90.

[gelisam]

I guess I should explain why I removed it in the first place.

I want processSpec and processArgs to have the same behaviour. If we check and create the context folder inside parseArgs, then processSpec will not also check and create the context folder, adding a burden to the users of processSpec (currently, that's just tests, but we might like to expose Hawk as a library someday).

The reason OptionParser uses Uncertain is so that warnings and errors may be raised while conversion from [String] to HawkSpec. When you suggested it, I thought it was a good idea to check whether the files passed as argument as readable or writable, but when I saw that you were also creating the folder, that raised a red flag. And now I no longer think we should check that the argument is readable or writable, only that the String is a proper FilePath.

[melrief]

Ok, but I still think we should exit with the right error when the context directory hasn't the right permissions. IOExceptions on reading files can be problematic for the user, I think it is better to print a clean error when some IO actions doesn't work as expected. That's why I have done the check in that way.

[gelisam]

I agree. Let's add checkContextDir back, but somewhere inside processSpec this time.

[gelisam]

Since the test is explicitly checking that the parser is raising the warning, and we no longer want the parser to raise that warning, I have simply removed the part of the test which checks for the warning.

I guess we'll have to wait for #26 in order to properly test that the warning is emitted.

[gelisam]

Merged into develop, closing.

[gelisam]

Forgot the last commit.