Mailing-List: contact jdo-dev-help@db.apache.org; run by ezmlm
Precedence: bulk
Reply-To: jdo-dev@db.apache.org
Message-ID: <32682137.1188753559262.JavaMail.jira@brutus>
Date: Sun, 2 Sep 2007 10:19:19 -0700 (PDT)
From: "Craig Russell (JIRA)" <jira@apache.org>
To: jdo-dev@db.apache.org
Subject: [jira] Commented: (JDO-521) Create tests for relationship mapping
 (spec section 15.3)
In-Reply-To: <18770819.1187734830541.JavaMail.jira@brutus>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


    [ https://issues.apache.org/jira/browse/JDO-521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12524375 ] 

Craig Russell commented on JDO-521:
-----------------------------------

In AbstractRelationshipTest, 
should probably promote method deferredAssertTrue to JDO_Test, and should probably promote field isTestToBePerformed to JDO_Test.

In Relationship1To1NoRelationships, testSetToNewFromMappedSide and testSetToNewFromMappedBySide test for the "previous relationship (emp1) not nulled in new pm"); but there is no previous relationship (four places).

[nit] the code dept1.getEmployees().contains((Object)emp1)) should not cast emp1 to Object; there's no harm but it doesn't do anything so it's a distraction.

In Relationship1ToManyAllRelationships, testSetToExistingFromMappedSide
+            deferredAssertTrue(!dept1.getEmployees().contains(emp1),
+                ASSERTION_FAILED + testMethod,
+                "Postcondition is false; "
+                + "previous relationship (dept1) not nulled on flush");
The comment is incorrect. Perhaps "reference emp1 not removed from previous relationship (dept1.employees)"

In Relationship1ToManyAllRelationships, testAddExistingFromMappedbySide replaces the collection. We need a different test that instead of:
+            Set emps = new HashSet();
+            emps.add(emp4);
+            dept1.setEmployees(emps);
+            pm.flush();

does:
+            Set emps = dept1.getEmployees();
+            emps.add(emp4);
+            pm.flush();

The existing test testAddExistingFromMappedbySide should change as above, and add new tests:
testReplaceFromMappedBySide that does as above, creates a new HashSet and replaces the existing collection
testAddNewFromMappedBySide that adds a new Employee to the existing collection

General suggestion on the test naming: Add should add to an existing collection; Remove should remove from an existing collection; Replace should replace an existing collection (or null collection); SetNull should replace an existing collection with null.

In RelationshipManyToManyAllRelationships, need a tests testAddExistingFromMappedSide, testAddNewFromMappedSide, testAddExistingFromMappedBySide, testAddNewFromMappedBySide. Instead of replacing the collection, these tests would use the existing collection and add a new Employee or Project or add an existing Employee or Project.

In RelationshipManyToManyNoRelationships the comments on test naming also apply.
Is testAddExistingMFromMappedbySide a typo?

In RelationshipNegative1ToManyTest, testAddToMoreThanOne appears to be a copy of another test, including javadoc.
testAddToMoreThanOne has the wrong javadoc.


> Create tests for relationship mapping (spec section 15.3)
> ---------------------------------------------------------
>
>                 Key: JDO-521
>                 URL: https://issues.apache.org/jira/browse/JDO-521
>             Project: JDO
>          Issue Type: Test
>          Components: tck2
>    Affects Versions: JDO 2 maintenance release 1
>            Reporter: Michelle Caisse
>            Assignee: Michelle Caisse
>         Attachments: JDO-521.patch
>
>
> We need a set of tests to test that an implementation syncs both sides of a relationship on flush().

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.