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.
>
>
Index: dspace-test/src/test/java/org/dspace/content/CommunityTest.java
===================================================================
--- dspace-test/src/test/java/org/dspace/content/CommunityTest.java	(revision 5307)
+++ dspace-test/src/test/java/org/dspace/content/CommunityTest.java	(working copy)
@@ -140,6 +140,7 @@
             {
                 AuthorizeManager.authorizeActionBoolean((Context) any, (Community) any,
                         Constants.ADD); result = true;
+                AuthorizeManager.isAdmin((Context) any); result = true;
             }
         };
 
@@ -170,6 +171,7 @@
             {
                 AuthorizeManager.authorizeActionBoolean((Context) any, (Community) any,
                         Constants.ADD); result = false;
+                AuthorizeManager.isAdmin((Context) any); result = false;
             }
         };
 
@@ -179,6 +181,27 @@
     }
 
     /**
+     * Test of create method, of class Community.
+     */
+    @Test(expected=AuthorizeException.class)
+    public void testCreateNoAuth2() throws Exception
+    {
+        new NonStrictExpectations()
+        {
+            AuthorizeManager authManager;
+            {
+                AuthorizeManager.authorizeActionBoolean((Context) any, (Community) any,
+                        Constants.ADD); result = true;
+                AuthorizeManager.isAdmin((Context) any); result = false;
+            }
+        };
+
+        //community with no parent can't be created if we are not admin
+        Community created = Community.create(null, context);
+        fail("Exception expected");
+    }
+
+    /**
      * Test of findAll method, of class Community.
      */
     @Test
@@ -820,6 +843,7 @@
                         Constants.ADD); result = null;
                 AuthorizeManager.authorizeActionBoolean((Context) any, (Community) any,
                         Constants.ADD); result = true;
+                AuthorizeManager.isAdmin((Context) any); result = true;
             }
         };
 
@@ -926,6 +950,7 @@
                         Constants.ADD); result = true;
                 AuthorizeManager.authorizeAction((Context) any, (Community) any,
                         Constants.REMOVE); result = null;
+                AuthorizeManager.isAdmin((Context) any); result = true;
             }
         };
 
@@ -1019,8 +1044,7 @@
         {
             AuthorizeManager authManager;
             {
-                AuthorizeManager.authorizeActionBoolean((Context) any, (Community) any,
-                        Constants.ADD); result = true;
+                AuthorizeManager.isAdmin((Context) any); result = true;
             }
         };
 
------------------------------------------------------------------------------
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

Reply via email to