- Notifications
You must be signed in to change notification settings - Fork 378
DATAJDBC-256 - Run integration tests with Oracle. #232
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
mp911de left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a few comments. That ResultSet.getObject(…) returns driver-internal types is unfortunate. We could make use of JdbcUtils.getResultSetValue(…) and the actual property type to delegate result value conversion to the driver is we can make sure the requested type is a simple JDBC type (e.g. by verifying against JdbcSimpleTypes.HOLDER.isSimpleType(…)) otherwise, fall back to ResultSet.getObject(…). WDYT?
| @Test // DATAJDBC-112 | ||
| public void saveAndLoadAnEntityWithReferencedEntityById() { | ||
| | ||
| features.supportsQuotedIds(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we please switch to an annotation-based model (e.g. @EnabledOnFeature(Features.QUOTED_IDS))? All these calls to features obfuscate the test code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting. I have the very opposite impression. The direct call to features is shorter both in the underlying implementation and on the use case site and easier to understand.
I'll prepare a commit switching to a rule based approach.
...-jdbc/src/test/java/org/springframework/data/jdbc/testing/OracleDataSourceConfiguration.java Outdated Show resolved Hide resolved
...-relational/src/main/java/org/springframework/data/relational/core/dialect/IdGeneration.java Show resolved Hide resolved
...-relational/src/main/java/org/springframework/data/relational/core/dialect/IdGeneration.java Outdated Show resolved Hide resolved
| </systemPropertyVariables> | ||
| </configuration> | ||
| </execution> | ||
| <execution> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Container startup fails with a TNS error message quite regularly. That causes a lot of the CI builds to fail.
I'm wondering whether we should run DB2 and Oracle tests only in a nightly job.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where do you see these regular failures? In Jenkins I see one failure in the last 14 builds which would gives me hope that just waiting a little longer might fix the problem, especially since it is a TNS failure and not a "your account has been locked".
Sounds very promising. I didn't know that method! But I'd leave that to DATAJDBC-569 and limit this PR to the just getting the very basics working as agreed before if that is ok. |
...relational/src/main/java/org/springframework/data/relational/core/dialect/OracleDialect.java Outdated Show resolved Hide resolved
Original pull request: #223.
An Oracle database is started in Docker using Testcontainers. Tests that don't work yet are skipped using the TestDatabaseFeatures which allows central control, which database supports which feature. A suitable oracle image is deployed on DockerHub as "springci/spring-data-oracle-xe-prebuild:18.4.0". The official Oracle docker images as described here https://github.com/oracle/docker-images/blob/master/OracleDatabase/SingleInstance/README.md are not suitable since it needs about 15min to start and also TestContainers seems to break it by trying to us it to early. The referenced image was created based on these instructions: https://github.com/oracle/docker-images/tree/master/OracleDatabase/SingleInstance/samples/prebuiltdb The following features don't yet work for Oracle: * Loading of date like properties. * Ids with custom names. * Array support. * Boolean properties. * BigDecimals and BigInteger is limited to what fits into Oracles NUMBER data type. * Entities that use a join during load time, i.e. with a one to one relationship.
Rename IdGeneration method and added Javadoc. Fixes copyright statement in OracleDataSourceConfiguration. Fixes JavaDoc comment for OracleDialect. Increases the maximum wait time for the Oracle database to start.
35b4dd6 to 8089b2c Compare | @mp911de I left the migration to annotation based assumptions in a separate commit. I also rebased on the current master and removed the alias for join with as since that is already resolved. For the CI build stability I increased the maximum wait time to 5minutes for the Oracle db to start up. |
An Oracle database is started in Docker using Testcontainers. Tests that don't work yet are skipped using the TestDatabaseFeatures which allows central control, which database supports which feature. A suitable oracle image is deployed on DockerHub as "springci/spring-data-oracle-xe-prebuild:18.4.0". The official Oracle docker images as described here https://github.com/oracle/docker-images/blob/master/OracleDatabase/SingleInstance/README.md are not suitable since it needs about 15min to start and also TestContainers seems to break it by trying to us it to early. The referenced image was created based on these instructions: https://github.com/oracle/docker-images/tree/master/OracleDatabase/SingleInstance/samples/prebuiltdb The following features don't yet work for Oracle: * Loading of date like properties. * Ids with custom names. * Array support. * Boolean properties. * BigDecimals and BigInteger is limited to what fits into Oracles NUMBER data type. * Entities that use a join during load time, i.e. with a one to one relationship. Original pull request: #232.
Rename RequiredFeature to EnabledOnFeature to align with JUnit terminology. Use RunWith instead of Spring rules. Reformat code. Original pull request: #232.
| That's merged and polished now. |
Rename RequiredFeature to EnabledOnFeature to align with JUnit terminology. Use RunWith instead of Spring rules. Reformat code. Update documentation. Original pull request: #232.
…cessException if row does not exist. We now emit a TransientDataAccessException if an object with a provided Id yields no affected rows. Such an arrangement is typically an indicator for a bug where calling code expects the object to be inserted with a provided Id.
An Oracle database is started in Docker using Testcontainers.
Tests that don't work yet are skipped using the TestDatabaseFeatures which allows central control, which database supports which feature.
A suitable oracle image is deployed on DockerHub as "springci/spring-data-oracle-xe-prebuild:18.4.0".
The official Oracle docker images as described here https://github.com/oracle/docker-images/blob/master/OracleDatabase/SingleInstance/README.md are not suitable since it needs about 15min to start and also TestContainers seems to break it by trying to us it to early.
The referenced image was created based on these instructions: https://github.com/oracle/docker-images/tree/master/OracleDatabase/SingleInstance/samples/prebuiltdb
The following features don't yet work for Oracle:
See DATAJDBC-256.