[Data Manager] Tests assume you can't write to /
it seems that there are a few datamanager tests that assume they won't be able to write to '/' and thus fail if files can in fact be written (e.g. TestPutAndGetBlocks_ErrorDuringGetCollectionsBadWriteTo expects to not be able to write to /badwritetofile and TestPutAndGetBlocks_ErrorDuringGetCollectionsBadHeapProfileFilename expects to not be able to write to /badheapprofile).
It would be better to ensure the failure by creating an explicit conflict. Perhaps the test could create a "bad" subdir in the test tmp directory and `chmod 000` it, then put the bad files under that directory.
#7 Updated by Radhika Chippada over 5 years ago
Reviewing branch wtsi-hgi-8508-datamanager-test-badpaths:
- It appears that we can ensure the dir is not accessible by simply replacing "/badwritetofile" with "/nosuchdir/badwritetofile" or fmt.Sprintf("/nosuchdir%v/badwritetofile", time.Since(time.Now()).Nanoseconds()) rather than creating a tempdir and appending a string to it? Please consider this simplified approach instead.
testOldBlocksNotDeletedOnDataManagerError(t, fmt.Sprintf("/nosuchdir%v/badwritetofile", time.Since(time.Now()).Nanoseconds()), "", true, true)
- Also, please run go fmt
#8 Updated by Joshua Randall over 5 years ago
The simplified approach "/nosuchdir/badwritetofile" would probably work in most cases but it is trivial to break it by creating a "/nosuchdir". This realistically could occur accidentally, so I do not favor this option.
The slightly less simplified approach `fmt.Sprintf("/nosuchdir%v/badwritetofile", time.Since(time.Now()).Nanoseconds())` would almost definitely work in every conceivable scenario although it would still theoretically be possible to break it by creating a large number of /nosuchdir%v paths with future times. I agree this is extremely unlikely to ever be an issue unless someone is "attacking" the test intentionally.
The proposed approach (creating a temp dir that we "own" and then referring to a nonexistent path within it) should always work unless some concurrent process creates the bad path in our temporary directory after we create the temporary directory (i.e. an active attack against the test).
I went for the proposed approach because it is the safest thing I could think of. If you are concerned about the extra temporary directory creation, I'm happy to compromise on your timestamp based solution.