[
Date Prev][
Date Next][
Thread Prev][
Thread Next][
Date Index][
Thread Index]
[
List Home]
Re: [eclipselink-dev] rewriting updateall queries clean-up code in tests
|
Hi Guys,
A few comments inline:
Andrei Ilitchev wrote:
Hi Dies,
Two questions about the suggested patch.
1. Sequencing.
My understanding that Symfoware platform doesn't allow SEQUENCE to be
used as a table name,
but rather than changing all the tests that use AUTO and TABLE sequences
(and that work on all platforms other than Symfoware)
please consider re-defining default sequence table name by defining
sequence generators with magic names: "SEQ_GEN" and "SEQ_GEN_TABLE".
The sequence defined by SEQ_GEN will be used for AUTO;
the sequence defined in SEQ_GEN_TABLE will be used for table sequence in
cases like:
@GeneratedValue(strategy=GenerationType.TABLE,
generator="CMP3_AAA_GENERATOR")
when no CMP3_AAA_GENERATOR is defined.
In general, I agree with Andrei that we shouldn't stop testing table sequences
with the name SEQUENCE for all platforms - it has been our default for a long
time so most users will have it. The issue here will be making changes that fit
into our test framework.
In talking with Andrei, I think a better solution may be to implement a
customizer that loops through the sequences in a project and changes any table
sequence with the name "SEQUENCE" to something else. That customizer could be
enabled with a System property when running the tests on Symfoware.
2. UpdateAll/DeleteAll
Please consider instead of removing all usages of updateAll/deleteAll
queries in the tests clean up code adding a flag
supportsModifyAllQueries (or smth like that) to the platform
(or alternatively adding a persistence unit property?)
and keeping the original updateAll/deleteAll unless the platform doesn't
support them
I am not a fan of adding a method to DatabasePlatform that will just be used for
testing.
I can see the argument that we should not alter our testing for one platform,
and that there may be some testing that occurs as a side-effect of these
clean-up methods. When I, however, look at the code we are changing, nearly all
of them exercise a DeleteAll query on a Basic field on Employee. In my opinion,
we have exercised that particular query adequately in the DeleteAll-specific
tests and the changes Dies has provided should be fine. There are a small
number these changes I have questions about, but I will be able to provide more
detail when I investigate integrating the final patch.
My vote is that we keep the majority of Dies' updates - but I am open to
arguments that go the other way.
<snip>
I also noticed such queries in some other tests and wasn't sure
whether/how to fix them. What do you think?
*
jpa/eclipselink.jpa.test/src/org/eclipse/persistence/testing/tests/jpa/advanced/ReportQueryAdvancedJUnitTest.java
The tests here don't seem to use UpdateAll/DeleteAll queries but the
clear method does. It uses UnitOfWork instead of EntityManager. Where
can I find the UnitOfWork API equivalent to doing a find and removing
each object in the resultset?
I am a hesitant to change this test for all platforms, but you could add an
isSymfoware() check and then do some alternate clean up. Which queries fail?
(I assume just the ones that deal with Employee)
Here is some pseudo code:
Iterator<Employee> emps = uow.readAllObjects(Employee.class).iterator()
while (emps.hasNext()){
Employee emp = emps.next();
emp.setManager(null);
emp.setAddress(null);
uow.deleteObject(emp);
};
*
jpa/eclipselink.jpa.test/src/org/eclipse/persistence/testing/tests/jpa/advanced/JoinedAttributeAdvancedJunitTest.java
Same as above, UpdateAll/DeleteAll with UOW in clear method. Here I
think the following tests rely on UpdateAll/Delete queries so need to be
excluded for Symfoware, would you agree?
- testTwoUnrelatedResultWithOneToManyJoins
- testMultipleUnrelatedResultWithOneToManyJoins
- testTwoUnrelatedResultWithOneToOneJoins
- testTwoUnrelatedResultWithOneToOneJoinsWithExtraItem
The clear should be able to be fixed in much the same way as above.
I agree those test should be excluded for symfoware.
*
jpa/eclipselink.jpa.test/src/org/eclipse/persistence/testing/tests/jpa/fieldaccess/advanced/JoinedAttributeAdvancedJunitTest.java
Same as above.
*
jpa/eclipselink.jpa.test/src/org/eclipse/persistence/testing/tests/jpa/jpql/JUnitJPQLComplexTestSuite.java
complexConditionCaseInUpdateTest
This test fails because it uses a multi-table entity. If I would rewrite
it to use an entity that is mapped to a simple table, it might pass.
Should I try, or should I just skip these type of tests on Symfoware?
Let's just skip this kind of test for Symfoware. Altering the test will reduce
our testing of multi-table.
The patch also includes a number of other issues I addressed. Could you
review them?
When we have come up with a good solution for the issue with SEQUENCE, I'll go
through the whole patch and either integrate it (with any small changes needed)
or provide feedback about bigger changes.
-Tom
Thanks,
Dies
_______________________________________________
eclipselink-dev mailing list
eclipselink-dev@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev
_______________________________________________
eclipselink-dev mailing list
eclipselink-dev@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev