Yevgeny Zaspitsky has posted comments on this change. Change subject: core: test that verifies DAO classes being annotated with Singleton ......................................................................
Patch Set 2: (2 comments) https://gerrit.ovirt.org/#/c/39572/2/backend/manager/modules/dal/pom.xml File backend/manager/modules/dal/pom.xml: Line 16: <dependencies> Line 17: <dependency> Line 18: <groupId>org.reflections</groupId> Line 19: <artifactId>reflections</artifactId> Line 20: <version>0.9.9-RC1</version> > why use that specific old version when you can use 0.9.9 ? Done Line 21: <scope>test</scope> Line 22: </dependency> Line 23: <dependency> Line 24: <groupId>commons-collections</groupId> https://gerrit.ovirt.org/#/c/39572/2/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/DaoCdiIntegrationTest.java File backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/DaoCdiIntegrationTest.java: Line 28: Line 29: for (Class<? extends DAO> dao : daos) { Line 30: if (isConcreteClass(dao)) { Line 31: assertTrue("A concrete DAO class has to be annotated with @Singleton: " + dao.getCanonicalName(), Line 32: dao.isAnnotationPresent(Singleton.class)); > shouldn't it also be annotated with @Named ? 1. AFAIK @Named was added for Spring only as the last does not recognize @Singleton properly (or at all). So that's for tests only and does not affect production run-time as far as we use that with default value. 2. I wouldn't use @Named for that purpose and I'm in favor of removing them from the production code and replace by some configuration code or XML in tests. 3. If they wouldn't exist an exsistent DAO test that relies on their existence would fail anyhow. That might give you the hint about my opinion on verifying that existence. Line 33: } Line 34: } Line 35: } Line 36: -- To view, visit https://gerrit.ovirt.org/39572 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8057dfce430cb3ce6a35181479b8999b4a1fe264 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yevgeny Zaspitsky <yzasp...@redhat.com> Gerrit-Reviewer: Moti Asayag <masa...@redhat.com> Gerrit-Reviewer: Yevgeny Zaspitsky <yzasp...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
