Missing addTearDownClass in org.apache.jdo.tck.query.jdoql.variables.VariablesWithoutExtent

• Key: JDO-423
• Type: Bug
• Status: Closed
• Resolution: Invalid
• Priority: Minor
• Assignee: Craig Russell
• Reporter: Ilan Kirsh
• Component/s: tck2
• Affects Version/s: JDO 2 final

Description

Lines 101- 108, instead of:

    protected void localSetUp() {
        addTearDownClass(CompanyModelReader.getTearDownClasses());
        loadAndPersistCompanyModel(getPM());
        NoExtent noExtent = new NoExtent(1);
        makePersistent(noExtent);
        addTearDownInstance(noExtent);
    }

should be:

    protected void localSetUp() {
        addTearDownClass(CompanyModelReader.getTearDownClasses());
        addTearDownClass(NoExtent.class); // Added missing addTearDownClass
        loadAndPersistCompanyModel(getPM());
        NoExtent noExtent = new NoExtent(1);
        makePersistent(noExtent);
        addTearDownInstance(noExtent);
    }

Comments

[Michael Bouschen]

Method localSetUp registers the NoExtent instance for removal during tearDown by calling addTearDownInstance(noExtent) at the end. We cannot use method addTearDownClass for class NoExtent. Method tearDown gets the extent for all classes registered using addTearDownClass and this would fail for class NoExtent, because the metatdata defines requires-extent="false".

I propose to close this issue as invalid. What do you think?

[Ilan Kirsh]

This issue still reflects a real (minor) problem, even though it is clear to me now that the solution that I suggested is invalid. I noticed that after a failure to delete the NoExtent instance once this test fails for all the next runs. Therefore, an alternative solution is still required.

Please forgive my ignorance, my familiarly with the TCK architecture is very limited. However, I will try another suggestion - maybe moving the addTearDownInstance to the beginning of the method is the solution? Maybe putting it in a finally block?

[Michael Bouschen]

Hi Ilan,

you input is highly appreciated.

What kind of issue are you running into with cleaning up the NoExtent instances? I'm not sure whether moving addTearDownInstance to the beginning of the method would make any difference. Method addTearDownInstance retrieves the oid of the parameter instance and stores it in a local set. At cleanup time method tearDown iterates the list of registered oids, retrieves the pc instances using getObjectById and calls pm.deletePersistent to remove the instance. But if the pm.deletePersistent fails, the TCK is in trouble, because it cannot cleanup the database to prepare the next run. Do you have an idea what to do?

Regards Michael

[Ilan Kirsh]

Thanks Michael.

I tried several runs of the TCK tests that ObjectDB already passes with application identity.

1st run (ordinary) - All passed
2nd run - with the implementation of deletePersistentAll in a comment - 134 failed
3rd run (ordinary) - 52 failed (because of undeleted objects from the previous run)
4th run (ordinary) -1 fail
5th run (ordinary) -1 fail

The one that keeps failing is VariablesWithoutExtent and the error message is: "instance with key '1' already exists". Finally I could pass also this test again without deleting the database, using:

    protected void localSetUp() {
        addTearDownClass(CompanyModelReader.getTearDownClasses());

        loadAndPersistCompanyModel(getPM());
        NoExtent noExtent = new NoExtent(1);
        try {
            makePersistent(noExtent);
        }
        catch (JDOUserException x) {
            noExtent = (NoExtent)pm.getObjectById(
                new IntIdentity(NoExtent.class, 1));
        }
        addTearDownInstance(noExtent);
    }

I don't know if this workaround can fit every configuration and every implementation, but it should give the general idea of the problem.

Regards,

Ilan

[Craig Russell]

Michael and I looked at the test and can't see anything wrong to suggest changing the test.

As Michael noted above, the test does call addTearDownInstance in the setUp method, and during tearDown the instance will be deleted.

Note that this is the only class that uses the addTearDownInstance method. The other tests all use addTearDownClass.

To help debug this, you could trace the behavior of JDO_Test.deleteTearDownInstance which is called during tearDown of each test method. One thing I'd suggest is to look at the behavior of your implementation when getting the object id from an instance that has been created and made persistent in a transaction that has not yet committed.

[Ilan Kirsh]

I agree that the test is valid. Probably my expectations for automatic recovery after a delete failure in the implementation were too excessive. I just saw that all the other tests know to recover in the next run and this test is different. Maybe implementation of addTearDownInstance that deletes object by ID can solve this. However, I understand that this is not a job of the TCK and deletion of the database file solves the problem anyway, so I have no objection to close this issue as invalid.

[Craig Russell]

The TCK generally tries to clean up after each test, and if there are issues in the cleanup phase, then subsequent tests might fail. The cleanup depends on some basic operations working correctly, including delete of an individual instance and delete by extent. If these operations do not work, then they would need to be fixed before continuing.