Nir Soffer has posted comments on this change. Change subject: tests: Generalize sdm_indirection_tests ......................................................................
Patch Set 1: Code-Review-1 (2 comments) https://gerrit.ovirt.org/#/c/46259/1/tests/sdm_indirection_tests.py File tests/sdm_indirection_tests.py: Line 286: # The class that will be initiating redirection of calls. For example, Line 287: # we use a faked StorageDomain object as the source of redirection to Line 288: # StorageDomainManifest objects. Line 289: # This attribute must be specified by the child class. Line 290: indirectionSourceClass = None Lets shorten the names - sourceClass, targetName Line 291: Line 292: # The name of the destination class instance in the source class. For Line 293: # example, calls are redirected from StorageDomain objects to Line 294: # StorageDomainManifest objects and each StorageDomain contains an instance Line 296: # This attribute must be specified by the child class. Line 297: indirectionTargetName = None Line 298: Line 299: def setUp(self): Line 300: self.redirector = self.indirectionSourceClass() What if the source class needs arguments for constructing an instance? Better to keep setup in the concrete test class, and add the reusable part as a helper class used by the tests, not as mixin class. The tests can create the helper in setup: def setUp(self): self.checker = RedirectChecker(BlockStorageDomain("foo", "bar"), "_manifest") And the test will use the helper: def test_foo(self): self.checker.check_call("foo", ()) Now RedirectChecker is now a reusable class that can live in testlib. Line 301: Line 302: def _check(self, fn, args, result): Line 303: target = getattr(self.redirector, self.indirectionTargetName) Line 304: getattr(self.redirector, fn)(*args) -- To view, visit https://gerrit.ovirt.org/46259 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icb68cb27d4e36c22f74a1cc445f38bea4d0ce4da Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <[email protected]> Gerrit-Reviewer: Adam Litke <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
