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: araco...@gmail.com [mailto:araco...@gmail.com] 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 > <robin.tay...@ed.ac.uk> 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: araco...@gmail.com [mailto:araco...@gmail.com] 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 > > <robin.tay...@ed.ac.uk> 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:tdono...@duraspace.org] > > > 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. > > > > > -- 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 Dspace-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dspace-devel
