Skip to content

Migration to junit5#1776

Merged
velo merged 20 commits into
OpenFeign:masterfrom
kamilkrzywanski:master_ibm_junit5
Jun 8, 2026
Merged

Migration to junit5#1776
velo merged 20 commits into
OpenFeign:masterfrom
kamilkrzywanski:master_ibm_junit5

Conversation

@kamilkrzywanski

Copy link
Copy Markdown
Contributor

Hi! With openrewrie and little help of claude I've migratted all tests to junit 5. This migration exposed that some tests never ben run before. I've fixed connection strings and found 2 bugs in querydsl-libraries/querydsl-jpa/src/main/java/com/querydsl/jpa/support/DialectSupport.java and querydsl-tooling/querydsl-sql-codegen/src/main/java/com/querydsl/sql/codegen/MetaDataExporter.java

It's big review but lots of classes is just change of import...

@velo velo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking on the JUnit 5 migration. The CI is green, but I can't approve this yet because the production-code fixes in the PR need targeted tests.\n\nRequired changes:\n- querydsl-libraries/querydsl-jpa/src/main/java/com/querydsl/jpa/support/DialectSupport.java:86 adds a new null-return-type registration path, but querydsl-libraries/querydsl-jpa/src/test/java/com/querydsl/jpa/support/DialectSupportTest.java only changes the JUnit import. Please add a test that exercises a template whose type is null, so the Hibernate registry path is covered.\n- querydsl-libraries/querydsl-jpa/src/main/java/com/querydsl/jpa/support/QSQLServerDialect.java:21 adds a new public SQL Server dialect, but there is no test asserting the SQL Server functions it registers. Please add coverage for the new dialect behavior, including current_date.\n- querydsl-tooling/querydsl-sql-codegen/src/main/java/com/querydsl/sql/codegen/MetaDataExporter.java:413 changes JDBC column-read ordering for Oracle default values. The converted MetaDataExporter tests do not add a regression for a column with COLUMN_DEF/default metadata. Please add a focused regression test for that scenario, or a mocked ResultSet/metadata test that proves COLUMN_DEF is read before ORDINAL_POSITION.\n\nBackwards compatibility and security look fine from this pass; the blocker is the required coverage for the production changes.

kamilkrzywanski and others added 2 commits June 6, 2026 21:29
…en wrong config) + fixed bug with dialect functions registration and fix MetaDataExporter bug - velo requested changes

@velo velo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the follow-up. The new targeted coverage addresses the production-code changes: the null-type dialect registration path, SQL Server dialect function registration including current_date, and the MetaDataExporter COLUMN_DEF/ORDINAL_POSITION ordering regression are all covered. CI is green, and I do not see compatibility or security concerns.

@velo velo merged commit 83f956b into OpenFeign:master Jun 8, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants