Allon Mureinik has posted comments on this change.
Change subject: core: introduce osinfo service implementation over RMI
......................................................................
Patch Set 3: I would prefer that you didn't submit this
(5 inline comments)
Question about the unit tests:
As far as I can gather, they assume the libosinfo service(?) is up and running.
Is this a valid assumption? Won't we be failing tests for anyone who didn't
install it?
Not that I think it's a bad test - but it should be quarantined in its own
profile.
....................................................
File
backend/manager/tools/src/main/java/org/ovirt/engine/core/osinfo/jna/libosinfo/OsinfoResourcesList.java
Line 38: public static class ByReference extends OsinfoResourcesList
implements Structure.ByReference {
Line 39:
Line 40: };
Line 41: public static class ByValue extends OsinfoResourcesList
implements Structure.ByValue {
Line 42:
please use the project's formatter - this class is full of tabs instead of
spaces.
Line 43: }
Line 44:
....................................................
File
backend/manager/tools/src/main/java/org/ovirt/engine/core/osinfo/server/LibosinfoJNAServiceImpl.java
Line 12:
Line 13: private LibosinfoLibrary lib;
Line 14: private Pointer db;
Line 15:
Line 16: private String path = "/usr/share/libosinfo/db";
Should be private static final
Line 17:
Line 18: public LibosinfoJNAServiceImpl() {
Line 19: initLibraries();
Line 20: initOsDb();
....................................................
File
backend/manager/tools/src/test/java/org/ovirt/engine/core/osinfo/server/TestLib.java
Line 7: import org.junit.Test;
Line 8: import org.ovirt.engine.core.osinfo.server.LibosinfoJNAServiceImpl;
Line 9: import org.ovirt.engine.core.utils.osinfo.OSIdentifiers;
Line 10:
Line 11: //TODO use Junit's theory paradigm instead of Test methods
Indeed :-)
Line 12: public class TestLib {
Line 13:
Line 14: private static LibosinfoJNAServiceImpl impl;
Line 15: private static String arch;
Line 16:
Line 17: @BeforeClass
Line 18: public static void init() {
Line 19: impl = new LibosinfoJNAServiceImpl();
Line 20: arch = "x86_64";
if this is just a constant, inline it
Line 21: }
Line 22:
Line 23: @Test
Line 24: public void testRecommendedRam() {
Line 52: System.out.printf("minimum number of CPUs for os %s with arch
%s", Windows2008, arch);
Line 53: long minimumCpus =
impl.getMinimumNumberOfCpus(OSIdentifiers.from(Windows2008), arch);
Line 54: System.out.printf(": %s \n",
Line 55: minimumCpus);
Line 56: Assert.assertTrue(minimumCpus > 0);
Please use logs instead of System.out.
You can add a log4j configuration to redirect to stdout if you want to see
these printings.
Line 57: }
--
To view, visit http://gerrit.ovirt.org/12937
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia893bdc3dfc486e0b3e735f4e187996710a05f54
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Roy Golan <rgo...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skriva...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Roy Golan <rgo...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
