Antoni Segura Puimedon has posted comments on this change. Change subject: Test: regression test to catch cross imports. ......................................................................
Patch Set 19: I would prefer that you didn't submit this (2 inline comments) .................................................... File tests/crossImportsTests.py.in Line 27: Line 28: def path_without_vdsm_ext_mod(): Line 29: sub_dirs = ['client', 'vdsm', 'vdsm_api'] Line 30: exclude_dirs = [os.path.abspath( Line 31: os.path.join(os.path.dirname(__file__), '..', sub_dir)) Shouldn't exclude_dirs reference also /usr/share/vdsm (rather the macro for it), and such other installation directories to make sure that the interpreter does not get the modules from there? Line 32: for sub_dir in sub_dirs] Line 33: new_path = list(it.ifilterfalse( Line 34: lambda x: os.path.abspath(x) in exclude_dirs, sys.path)) Line 35: return new_path Line 30: exclude_dirs = [os.path.abspath( Line 31: os.path.join(os.path.dirname(__file__), '..', sub_dir)) Line 32: for sub_dir in sub_dirs] Line 33: new_path = list(it.ifilterfalse( Line 34: lambda x: os.path.abspath(x) in exclude_dirs, sys.path)) I'd rather have: return [path for path in sys.path if os.path.abspath(path) not in exclude_dirs] Line 35: return new_path Line 36: Line 37: Line 38: def get_mods(path): -- To view, visit http://gerrit.ovirt.org/14311 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I46b293c34e5968d21097137f3d1076f4199a5fd7 Gerrit-PatchSet: 19 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Giuseppe Vallarelli <gvall...@redhat.com> Gerrit-Reviewer: Antoni Segura Puimedon <asegu...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Giuseppe Vallarelli <gvall...@redhat.com> Gerrit-Reviewer: Livnat Peer <lp...@redhat.com> Gerrit-Reviewer: Vinzenz Feenstra <vfeen...@redhat.com> Gerrit-Reviewer: Zhou Zheng Sheng <zhshz...@linux.vnet.ibm.com> Gerrit-Reviewer: oVirt Jenkins CI Server _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
