Change in vdsm[master]: configurator doesn't load pyc files under configurators folder

nsoffer at redhat.com nsoffer at redhat.com
Wed Sep 9 16:00:39 UTC 2015


Nir Soffer has posted comments on this change.

Change subject: configurator doesn't load pyc files under configurators folder
......................................................................


Patch Set 5:

(3 comments)

https://gerrit.ovirt.org/#/c/45846/5/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:

Line 28: """
Line 29: 
Line 30: 
Line 31: from collections import deque
Line 32: import re
Sorting?
Line 33: import argparse
Line 34: import os
Line 35: import sys
Line 36: import traceback


Line 57:     return set(
Line 58:         os.path.splitext(name)[0]
Line 59:         for name in os.listdir(path)
Line 60:         if is_module(name)
Line 61:     )
Looks good
Line 62: 
Line 63: 
Line 64: _CONFIGURATORS = {}
Line 65: for module in _listmodules(configurators):


https://gerrit.ovirt.org/#/c/45846/5/tests/toolTests.py
File tests/toolTests.py:

Line 89: 
Line 90:         self.assertEquals(
Line 91:             configurator._listmodules(configurators),
Line 92:             expected
Line 93:         )
This does not test the inclusion of pyc files on node, and also will break when we add the next module.

I think the best way to test this is to create a temporary directory with some files e.g. ("a.py", "a.pyc, "_underscore.py", "_underscore.pyc"), and test that we get the correct set (e.g. set(["a"]))

I would add 4 tests:
- including py files
- including pyc files
- removing duplicates (py, pyc)
- excluding private modules
Line 94: 
Line 95:     def testPatch(self):
Line 96:         self.configurator = MockModuleConfigurator('a')
Line 97:         self.function_was_run = False


-- 
To view, visit https://gerrit.ovirt.org/45846
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia529de0069e2f4ec168a4b9df82ba62c56d66730
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourfali at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list