Inheritance3 : should check if the JDO implementation supports "javax.jdo.option.mapping.RelationSubclassTable"

• Key: JDO-311
• Type: Bug
• Status: Resolved
• Resolution: Fixed
• Priority: Major
• Assignee: Michael Watzek
• Reporter: Andy Jefferson
• Component/s: tck2
• Affects Version/s: JDO 2 beta
• Fix Version/s: JDO 2 final

Description

The inheritance3 test should check whether PMF.supportedOptions contains "javax.jdo.option.mapping.RelationSubclassTable" before running the test. JPOX doesn't support this option yet the test still runs (and fails)

Comments

Some inheritance configurations must specify property "jdo.tck.requiredOptions" as shown below:

- inheritance1.conf: javax.jdo.option.mapping.JoinedTablePerClass.
- inheritance2.conf: javax.jdo.option.mapping.NonJoinedTablePerConcreteClass, javax.jdo.option.mapping.RelationSubclassTable.
- inheritance3.conf: javax.jdo.option.mapping.JoinedTablePerConcreteClass, javax.jdo.option.mapping.RelationSubclassTable.

[Michael Watzek]

The attached patch fixes this issue.

Some comments:
I moved method isTestToBePerformed() to class JDOTest. Thus, other test classes besides CompletenessTest may check for required options if this is necessary.

[Craig Russell]

This patch looks good; just a couple of comments.

1. I'd suggest adding some more delimiters to:

       StringTokenizer tokenizer = new StringTokenizer(requiredOptions, " ,");

I'd change the delimiters to include semicolon, tab, newline, carriage return:

       StringTokenizer tokenizer = new StringTokenizer(requiredOptions, " ,;\n\r\t");

2. I think there is no need to remove the leading "org.apache.jdo.tck..." from the class name in the message, so these lines would not be needed.

+ int index = getClass().getName().lastIndexOf('.');
+ String testName = index==-1 ?
+ getClass().getName() : getClass().getName().substring(index+1);

But maybe I misunderstand your intent here.

[Michael Watzek]

I agree with 1. above.
The removal of the package prefix in 2. above is consistent with other TCK test cases: All calls of method "printUnsupportedOptionalFeatureNotTested" in TCK test classes pass a non-qualified classnames. For this reason, I propose to pass a non-qualified classname here either, or to adapt the calls in other TCK test cases as well.
Please let me know how to proceed.

[Craig Russell]

My preference is to print the entire class name in all places. But I would not hold up this JIRA to make them all consistent (time permitting, of course)

grep printUnsupportedOptionalFeatureNotTested `find src/java -name "*java"`
src/java/org/apache/jdo/tck/api/persistencecapable/AbstractPersistenceCapableTest.java: printUnsupportedOptionalFeatureNotTested(
src/java/org/apache/jdo/tck/api/persistencecapable/AbstractPersistenceCapableTest.java: printUnsupportedOptionalFeatureNotTested(
src/java/org/apache/jdo/tck/api/persistencemanager/cache/EvictingWithRetainValuesFalse.java: printUnsupportedOptionalFeatureNotTested("EvictingWithRetainValuesFalse", "javax.jdo.option.RetainValues");
src/java/org/apache/jdo/tck/JDO_Test.java: protected void printUnsupportedOptionalFeatureNotTested(
src/java/org/apache/jdo/tck/lifecycle/HollowInstanceMaintainsPK.java: printUnsupportedOptionalFeatureNotTested(
src/java/org/apache/jdo/tck/lifecycle/nontransactional/ModificationOfNontransactionalInstanceOutsideTransaction.java: printUnsupportedOptionalFeatureNotTested(
src/java/org/apache/jdo/tck/lifecycle/nontransactional/ModificationOfNontransactionalInstanceOutsideTransaction.java: printUnsupportedOptionalFeatureNotTested(
src/java/org/apache/jdo/tck/lifecycle/nontransactional/ModificationOfNontransactionalInstanceOutsideTransaction.java: printUnsupportedOptionalFeatureNotTested(
src/java/org/apache/jdo/tck/mapping/CompletenessTest.java: printUnsupportedOptionalFeatureNotTested(testName, token);

The usage in these five classes is inconsistent, so it's not a high priority to make them all consistent. It's worth a low priority JIRA to make them consistent, but I'd prefer that the first parameter to the printUnsupportedOptionalFeatureNotTested be the fully qualified class name.

If there is ambiguity (see org/apache/jdo/tck/lifecycle/nontransactional/ModificationOfNontransactionalInstanceOutsideTransaction) we should probably qualify the class name with a method name, e.g.:
org.apache.jdo.tck.lifecycle.nontransactional.ModificationOfNontransactionalInstanceOutsideTransaction.testSameInstance()

It's wordy, but unambiguous.

I have no preference to changing them all with this JIRA or opening another JIRA.

[Michael Watzek]

The second patch implements comments 1. and 2. above. Note, that the second patch makes all calls of printUnsupportedOptionalFeatureNotTested consistent. For this reason, it contains diffs for the following classes also:

- src\java\org\apache\jdo\tck\lifecycle\HollowInstanceMaintainsPK.java
- src\java\org\apache\jdo\tck\lifecycle\nontransactional\ModificationOfNontransactionalInstanceOutsideTransaction.java
- src\java\org\apache\jdo\tck\api\persistencemanager\cache\EvictingWithRetainValuesFalse.java
- src\java\org\apache\jdo\tck\api\persistencecapable\AbstractPersistenceCapableTest.java

[Craig Russell]

Looks great! I like that you promoted isTestToBePerformed to JDO_Test.

Just two comments.

1. Please use xxx.patch as the attachment name. Using patch2 as the file type means that my browser downloads the file (unknown type) and I have to find it and open it using a text editor. The second patch can be named e.g. JDO-311-2.patch.

2. Lines often exceed 80 characters. No need to shorten existing lines but new/changed lines should be cut at 80.

[Michael Watzek]

The second patch has been checked in (revision 382739).