ShaoHe Feng has posted comments on this change.
Change subject: storage.misc: subclass DeferableContext for easy undoing
......................................................................
Patch Set 4: I would prefer that you didn't submit this
(2 inline comments)
....................................................
File tests/rollbackContextTests.py
Line 62: self.tempfiles(10, UndoError, rollback)
Line 63:
Line 64: self.assertRaises(UndoError, exceptionInUndo)
Line 65: # Directory and files should be removed
Line 66: self.assertEquals(glob.glob(self.tmpdirPrefix + "*"), [])
why need a wildcard?
glob.glob(self.tmpdirPrefix + "*") can not list the subdir or subfile.
if a file named self.tmpdirPrefix + 'another', this test can be failed.
surely, it is impossible for uuid.uuid4.
Line 67:
Line 68: def testExceptionUnderContext(self):
Line 69: def exceptionUnderContext():
Line 70: with RollbackContext() as rollback:
Line 82: for i in range(0, fileCount):
Line 83: with open(os.path.join(dirPath, str(i)), "rb") as f:
Line 84: self.assertEquals(int(f.read()), i)
Line 85: # Directory and files should be removed
Line 86: self.assertEquals(glob.glob(self.tmpdirPrefix + "*"), [])
I have check the RollbackContext code. you implement a mechanism that catch
the first Exception.
can you construct more testes that the Exception was raised during the
RollbackContext?
example:
So during the "with RollbackContext() as rollback", you can make a new file.
not add it's remove operation to undo. then os.rmdir(dirPath) will raise an
Exception.
with RollbackContext() as rollback:
dirPath = self.tempfiles(fileCount, None, rollback)
open(os.path.join(dirPath, "Newfile"), "rb")
Open a file, but not close it. then both os.remove(path) and
os.rmdir(dirPath) will raise an Exception.
with RollbackContext() as rollback:
dirPath = self.tempfiles(fileCount, None, rollback)
open(os.path.join(dirPath, str(5)), "rb")
just a suggestion.
--
To view, visit http://gerrit.ovirt.org/8671
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc932637dd81c3becf92de34ea647c1cea136111
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Zhou Zheng Sheng <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Royce Lv <[email protected]>
Gerrit-Reviewer: Ryan Harper <[email protected]>
Gerrit-Reviewer: Saggi Mizrahi <[email protected]>
Gerrit-Reviewer: ShaoHe Feng <[email protected]>
Gerrit-Reviewer: Vinzenz Feenstra <[email protected]>
Gerrit-Reviewer: Zhou Zheng Sheng <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
