Pere, Yes -- thanks for finding the fix! It looks like I was the one who actually caused this problem -- I must've neglected to run a full unit test when committing my AIP work (which went into Trunk just a few days after the Unit Test Framework).
I agree with Robin's point. We need to change our Committer practices so that we make sure to run through the Unit Tests *before* committing code (and we also need to be updating the Unit Tests when adding new code/methods). I'll write myself a note to bring this up at our next Developers Meeting. - Tim On 9/2/2010 5:25 AM, TAYLOR Robin wrote: > Hi Pere, > > Great work, thanks for that. I'll raise a Jira and attach the patch. > > This raises an issue for us all though, we already see code being committed > that breaks the tests. My concern is that we will see a gradual degradation > until people just ignore the test results. It must become standard practice > that people run the tests and get zero failures before committing any code. > Tests should be considered of equal importance to the code, if a test fails > then the code should be considered broken. That's enough ranting from me.... > > Cheers, Robin. > > > Robin Taylor > Main Library > University of Edinburgh > Tel. 0131 6513808 > >> -----Original Message----- >> From: [email protected] [mailto:[email protected]] On >> Behalf Of Pere Villega >> Sent: 02 September 2010 00:42 >> To: TAYLOR Robin >> Cc: Tim Donohue; dspace-devel Developers; Stuart Lewis >> Subject: Re: [Dspace-devel] Unit tests failures >> >> Hi all, >> >> I've run the tests from the code in the GSoC branch and I get >> no errors: >> >> Results : >> Tests run: 500, Failures: 0, Errors: 0, Skipped: 0 >> >> >> >> I downloaded the current trunk and I get: >> >> Results : >> >> Tests in error: >> testCreateAuth(org.dspace.content.CommunityTest) >> testEquals(org.dspace.content.CommunityTest) >> testAddSubcommunityAuth(org.dspace.content.CommunityTest) >> testRemoveSubcommunityAuth(org.dspace.content.CommunityTest) >> >> Tests run: 500, Failures: 0, Errors: 4, Skipped: 0 >> >> This seems to indicate the errors arise due to changes in the >> Community class. >> >> Issues detected: >> >> - testCreateAuth: >> >> I had to mock the authorization call: >> AuthorizeManager.authorizeActionBoolean((Context) any, >> (Community) any, Constants.ADD); >> >> I see that a new check seems to have been added in the >> Community.create(Community, Context, Handle), line 211: >> >> parent != null >> >> My tests try to create a community with no parent, and it >> fails when the code reaches that line, producing the error. >> I'm sure this change has a reason, but anyway I have to ask >> (as it breaks an existing test): it's intended? >> >> I've modified the test adding an extra method to test the >> case when we create a community without parent and we are not >> admins (should throw an exception according to new code). >> I've also added a mock of the "isAdmin" authorization check >> (I had only used the ADD access before assuming admin has all >> permissions, that was a mistake seeing the new code). >> >> >> - testEquals: >> - testAddSubcommunityAuth: >> - testRemoveSubcommunityAuth: >> >> Same issue as before, fixed the test code. >> >> An observation: when creating a Community, line 210 of class >> Community we check for: >> >> if (!(AuthorizeManager.isAdmin(context) || >> (parent != null&& >> AuthorizeManager.authorizeActionBoolean(context, parent, >> Constants.ADD)))) >> >> but when removing, line 1006, we only check for: >> >> AuthorizeManager.authorizeAction(ourContext, this, Constants.REMOVE); >> >> The second doesn't check if you are Admin. It's expected? Why >> yes on create and not on removal? Sorry but it confuses me. >> >> >> >> About the 2 tests related to performance, it would be good if >> everybody who is getting an error there could tweak the >> performance checks until they pass for them. That way we >> would get a sample of the minimum time expected in a >> developer machine to run them. Or we can remove them until a >> decision is made on them. >> >> >> As I'm not sure of what's my status with trunk since GSoC, I >> attach a patch with the changes done to CommunityTest. It >> should fix it. >> >> Any other issue, I'm here :) >> >> >> Cheers, >> Pere >> >> >> >> >> On Tue, Aug 31, 2010 at 9:20 AM, TAYLOR Robin >> <[email protected]> wrote: >> >> >> Hi Pere, >> >> Thanks for the help, there is no rush. >> >> I was thinking about the 'admin' failures eg... >> >> testEquals(org.dspace.content.CommunityTest) Time >> elapsed: 0.156 sec<<< ERROR! >> org.dspace.authorize.AuthorizeException: Only >> administrators can create communities >> >> Assuming the eperson is not an admin then this >> exception is an acceptable result and should be caught and >> ignored. I guess ideally we would want to test twice, once as >> an admin and once as an ordinary user, but I don't know how >> practical that is. >> >> I realise you have other commitments these days. I >> haven't touched the code as I didn't want to do so without >> checking with you first but let me know if you want me to do anything. >> >> Thanks again, Robin. >> >> >> >> Robin Taylor >> Main Library >> University of Edinburgh >> Tel. 0131 6513808 >> >> > -----Original Message----- >> >> > From: [email protected] [mailto:[email protected]] On >> > Behalf Of Pere Villega >> > Sent: 30 August 2010 18:02 >> > To: TAYLOR Robin >> > Cc: Tim Donohue; dspace-devel Developers >> > Subject: Re: [Dspace-devel] Unit tests failures >> > >> >> > Hi, >> > >> > I'll check the issues :) Can't assure I'll do it >> today, but ASAP. >> > >> > Cheers, >> > Pere >> > >> > >> > >> > On Mon, Aug 30, 2010 at 4:07 PM, TAYLOR Robin >> > <[email protected]> wrote: >> > >> > >> > Hi Tim, >> > >> > Thanks for the reply. I have had a quick look at the >> > failures that I get but you don't and they are performance >> > type tests which explains why its not consistent... >> > >> > Tests run: 2, Failures: 0, Errors: 2, Skipped: 0, Time >> > elapsed: 44.134 sec<<< FAILURE! >> > >> > testCountItems(org.dspace.content.CommunityCollectionIntegrati >> > onTest) Time elapsed: 29.797 sec<<< ERROR! >> > org.databene.contiperf.PerfTestFailure: Average >> > execution time of >> > org.dspace.content.CommunityCollectionIntegrationTest.testCoun >> > tItems exceeded the requirement of 333 ms, measured 593.86 ms >> > >> > The other failures are Exceptions thrown because the >> > eperson does not have admin authority. I'll track down Pere >> > and see if he can help and I'll raise a Jira for any >> required changes. >> > >> > Cheers, Robin. >> > >> > >> > >> > >> > >> > Robin Taylor >> > Main Library >> > University of Edinburgh >> > Tel. 0131 6513808 >> > >> > >> > > -----Original Message----- >> > > From: Tim Donohue [mailto:[email protected]] >> > > Sent: 30 August 2010 15:44 >> > > To: TAYLOR Robin >> > > Cc: dspace-devel Developers; Pere Villega >> > > Subject: Re: [Dspace-devel] Unit tests failures >> > > >> > > Hi Robin& all, >> > > >> > > I actually get different results as Robin >> when I run the >> > > following from [dspace-src]/dspace-test/ >> > > >> > > mvn package -Dmaven.test.skip=false >> > > >> > > I still get failures, but I only see four failures: >> > > >> > > Results : >> > > >> > > Tests in error: >> > > testEquals(org.dspace.content.CommunityTest) >> > > testCreateAuth(org.dspace.content.CommunityTest) >> > > >> testAddSubcommunityAuth(org.dspace.content.CommunityTest) >> > > >> > testRemoveSubcommunityAuth(org.dspace.content.CommunityTest) >> > > >> > > Tests run: 500, Failures: 0, Errors: 4, Skipped: 0 >> > > >> > > This is based on the absolute latest Trunk >> (revision 5302). >> > > >> > > Perhaps we should enter these issues into >> JIRA for review by >> > > Pere or whoever else can get to it? >> > > >> > > BTW -- I don't seem to be able to run 'mvn >> test' to run the >> > > tests, as described in Pere's wiki docs (it >> always returns >> > > 'tests are skipped'). >> > > I always have to run 'mvn package >> -Dmaven.test.skip=false' >> > > directly from the 'dspace-test' directory. >> > > >> > > - Tim >> > > >> > > >> > > On 8/30/2010 8:12 AM, TAYLOR Robin wrote: >> > > > I'm getting the following failures in the >> unit tests and >> > > wondered if anyone had already fixed them. >> I'm being lazy >> > > here so if I don't get any replies then I'll >> get my finger >> > > out and investigate. >> > > > >> > > > Cheers, Robin. >> > > > >> > > > >> > > > Results : >> > > > >> > > > Tests in error: >> > > > testEquals(org.dspace.content.CommunityTest) >> > > > testCreateAuth(org.dspace.content.CommunityTest) >> > > > >> testAddSubcommunityAuth(org.dspace.content.CommunityTest) >> > > > >> > testRemoveSubcommunityAuth(org.dspace.content.CommunityTest) >> > > > >> > > >> > >> testCountItems(org.dspace.content.CommunityCollectionIntegrationTest) >> > > > >> > > > >> > > >> > >> testCreateTree(org.dspace.content.CommunityCollectionIntegrationTest) >> > > > >> > > > Tests run: 500, Failures: 0, Errors: 6, Skipped: 0 >> > > > >> > > > >> > > > >> > > > Robin Taylor >> > > > Main Library >> > > > University of Edinburgh >> > > > Tel. 0131 6513808 >> > > >> > >> > -- >> > The University of Edinburgh is a charitable >> body, registered in >> > Scotland, with registration number SC005336. >> > >> > >> > >> > >> > >> >> -- >> >> The University of Edinburgh is a charitable body, registered in >> Scotland, with registration number SC005336. >> >> >> >> >> ------------------------------------------------------------------------------ This SF.net Dev2Dev email is sponsored by: Show off your parallel programming skills. Enter the Intel(R) Threading Challenge 2010. http://p.sf.net/sfu/intel-thread-sfd _______________________________________________ Dspace-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/dspace-devel
