| RE: AW: AW: [eclipselink-dev] regressions in the WDF test suite |
|
That seems fine. -----Original
Message----- Hi
James, the
ClassCastException is gone now. There are 6 issues remaining. I have created a
bug for this: https://bugs.eclipse.org/bugs/show_bug.cgi?id=312051 A
proposed fix attached. To make the tests positive, I just changed
assertInvalidQuery to assertValidQuery. Could
you please have a look? Thanks, Adrian Adrian Görler Von: James Sutherland
[mailto:JAMES.SUTHERLAND@xxxxxxxxxx] >> java.lang.ClassCastException: java.lang.String cannot be cast
to java.lang.Enum The test expects an error, and it getting one, so we could just broaden
the error allowed. We could change the error to throw some type of
ConversionException instead, but the class cast does seem to indicate pretty
clearly what is wrong. I think the JPA Query should be catching any errors a re-throwing as QueryException
or PersistenceException though, so this could probably be fixed. We could also just pass through the string values in the enum converter. >> I agree. Actually, I think this should have been done as part
of the changes causing the test failure. Who will do this? I am not familiar with these tests, or their framework, but can try to
update them. >> The WDF test suite contains many (~160) tests that assert that
invalid queries are rejected. Would you say that in general such tests are not suitable
for EclipseLink? I would leave them there. Negative tests are useful to ensure
things fail with good exceptions instead of badly. Obviously if the test is testing something that is valid, but just not
supported yet, when it is supported, then it will need to be changed to a
positive test. >> I understand that EclipseLink is deferring type checking to the
database now. No, EclipseLink has always done automatic type conversion within its
Expressions. There were just some (not all) cases in JPQL where typing
errors could be thrown. In _expression_ queries if you do "where id = 5" and id was a
long, EclipseLink would automatically convert the int to long, instead of
throwing a typing error. JPA, JPQL and EclipseLink have always supported more than is supported
by every database. For example CASE is not supported on Derby, but is
allowed in JPQL. We do our best to isolate any database differences or
limitations in our DatabasePlatform abstraction, but there will always be some things not supported on some
databases. Most things however are supported on most databases, so we
should not prevent users having access to this functionality just because one
database does not support it. -----Original
Message----- Hi
James, Ø
All of these failures seem to be negative tests that now pass. Not
quite. The tests TestConditionalExpression.testBetweenHandling3/10 fail in
createQuery with a ClassCastException. The query executed is "SELECT
p FROM Person p WHERE p.city.type BETWEEN 'bla' AND 'bla'" City.type
is an enum. Admittedly,
the query is nonsense and the tests expects it to fail. But I’d consider a
ClassCastException an issue. java.lang.ClassCastException:
java.lang.String cannot be cast to java.lang.Enum
at
org.eclipse.persistence.mappings.converters.EnumTypeConverter.convertObjectValueToDataValue(EnumTypeConverter.java:137)
at
org.eclipse.persistence.mappings.foundation.AbstractDirectMapping.getFieldValue(AbstractDirectMapping.java:808)
at
org.eclipse.persistence.internal.expressions.QueryKeyExpression.getFieldValue(QueryKeyExpression.java:372)
at
org.eclipse.persistence.internal.expressions.ConstantExpression.printSQL(ConstantExpression.java:122)
at
org.eclipse.persistence.expressions.ExpressionOperator.printCollection(ExpressionOperator.java:1922)
at
org.eclipse.persistence.internal.expressions.FunctionExpression.printSQL(FunctionExpression.java:447)
at org.eclipse.persistence.expressions.ExpressionOperator.printDuo(ExpressionOperator.java:1962)
at
org.eclipse.persistence.internal.expressions.CompoundExpression.printSQL(CompoundExpression.java:277)
at org.eclipse.persistence.internal.expressions.ExpressionSQLPrinter.translateExpression(ExpressionSQLPrinter.java:306)
at
org.eclipse.persistence.internal.expressions.ExpressionSQLPrinter.printExpression(ExpressionSQLPrinter.java:129)
at org.eclipse.persistence.internal.expressions.SQLSelectStatement.printSQL(SQLSelectStatement.java:1451)
at
org.eclipse.persistence.internal.databaseaccess.DatabasePlatform.printSQLSelectStatement(DatabasePlatform.java:2823)
at org.eclipse.persistence.platform.database.MySQLPlatform.printSQLSelectStatement(MySQLPlatform.java:618)
at
org.eclipse.persistence.internal.expressions.SQLSelectStatement.buildCall(SQLSelectStatement.java:755)
at org.eclipse.persistence.internal.expressions.SQLSelectStatement.buildCall(SQLSelectStatement.java:765)
at
org.eclipse.persistence.descriptors.ClassDescriptor.buildCallFromStatement(ClassDescriptor.java:671)
at org.eclipse.persistence.internal.queries.StatementQueryMechanism.setCallFromStatement(StatementQueryMechanism.java:386)
at
org.eclipse.persistence.internal.queries.StatementQueryMechanism.prepareSelectAllRows(StatementQueryMechanism.java:312)
at
org.eclipse.persistence.internal.queries.ExpressionQueryMechanism.prepareSelectAllRows(ExpressionQueryMechanism.java:1557)
at
org.eclipse.persistence.queries.ReadAllQuery.prepareSelectAllRows(ReadAllQuery.java:674)
at org.eclipse.persistence.queries.ReadAllQuery.prepare(ReadAllQuery.java:621)
at
org.eclipse.persistence.queries.DatabaseQuery.checkPrepare(DatabaseQuery.java:508)
at org.eclipse.persistence.queries.ObjectLevelReadQuery.checkPrepare(ObjectLevelReadQuery.java:733)
at
org.eclipse.persistence.queries.DatabaseQuery.prepareCall(DatabaseQuery.java:1575)
at
org.eclipse.persistence.internal.jpa.EJBQueryImpl.buildEJBQLDatabaseQuery(EJBQueryImpl.java:247)
at
org.eclipse.persistence.internal.jpa.EJBQueryImpl.buildEJBQLDatabaseQuery(EJBQueryImpl.java:180)
at
org.eclipse.persistence.internal.jpa.EJBQueryImpl.<init>(EJBQueryImpl.java:132)
at org.eclipse.persistence.internal.jpa.EJBQueryImpl.<init>(EJBQueryImpl.java:116)
at
org.eclipse.persistence.internal.jpa.EntityManagerImpl.createQuery(EntityManagerImpl.java:1348)
at org.eclipse.persistence.testing.tests.wdf.jpa1.query.QueryTest.assertInvalidQuery(QueryTest.java:167)
at
org.eclipse.persistence.testing.tests.wdf.jpa1.query.TestConditionalExpressions.testBetweenHandling3(TestConditionalExpressions.java:403)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
at
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
at java.lang.reflect.Method.invoke(Method.java:597)
at
org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:44)
at
org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:15)
at
org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:41)
at
org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:20)
at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:28)
at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:31)
at
org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:73)
at org.eclipse.persistence.testing.framework.wdf.SkipBugzillaTestRunner.runChild(SkipBugzillaTestRunner.java:176)
at
org.eclipse.persistence.testing.framework.wdf.SkipBugzillaTestRunner.runChild(SkipBugzillaTestRunner.java:37)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:180)
at org.junit.runners.ParentRunner.access$000(ParentRunner.java:41)
at org.junit.runners.ParentRunner$1.evaluate(ParentRunner.java:173)
at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:28)
at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:31)
at org.junit.runners.ParentRunner.run(ParentRunner.java:220)
at
org.eclipse.persistence.testing.framework.wdf.SkipBugzillaTestRunner.run(SkipBugzillaTestRunner.java:49)
at
org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:46)
at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
at
org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:467)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:683)
at
org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:390)
at
org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:197) -
We enhanced our JPQL support to allow a large number of valid queries,
or defer to the database to decide if the query is valid or not. -
The tests should either be removed, or change to verify they JPQL works,
instead of fails. I
agree. Actually, I think this should have been done as part of the changes
causing the test failure. Who will do this? The
WDF test suite contains many (~160) tests that assert that invalid queries are
rejected. Would you say that in general such tests are not suitable for EclipseLink? I
still don’t have a clear understanding of the guarantees given by the query
validation in EclipseLink. I
have learned that EclipseLink supports a wider range of queries as standard
JPQL. Also, I understand that EclipseLink is deferring type checking to the
database now. I think this makes a lot of sense if you consider the ORM mapper
aspect of EclipseLink only. But
considering JPA and JPQL a means for database-abstraction, from my point
of view, deferring validation to the database will hamper the portability.
Therefore I am not sure if deferring the checks to the DB is the right
approach. I think there is much value in strong type checking. I’d have
preferred explicit casts to leverage type conversion on the database. -Adrian Adrian Görler Von:
eclipselink-dev-bounces@xxxxxxxxxxx [mailto:eclipselink-dev-bounces@xxxxxxxxxxx]
Im Auftrag von James Sutherland We
enhanced our JPQL support to allow a large number of valid queries, or defer to
the database to decide if the query is valid or not. All
of these failures seem to be negative tests that now pass. This is
intended. The tests should either be removed, or change to verify they
JPQL works, instead of fails. -----Original
Message----- Hi, I
am observing regression in the WDF test suite: (This
is actually a zip file containing the html-junit-report). Please
check, whoever did changes in the BETWEEN or GROUP BY realm. -Adrian Adrian Görler |