Skip to content

Conversation

@VladoKuruc
Copy link
Contributor

Fixes for some of the Informix tests


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.


@hibernate-github-bot
Copy link

hibernate-github-bot bot commented Sep 3, 2024

Thanks for your pull request!

This pull request does not follow the contribution rules. Could you have a look?

❌ All commit messages should start with a JIRA issue key matching pattern HHH-\d+
    ↳ Offending commits: [449fa85]

› This message was automatically generated.

@mbellade
Copy link
Member

mbellade commented Sep 3, 2024

I'm not sure we want to spread special cases across the main test suite for a community-supported dialect.

@VladoKuruc
Copy link
Contributor Author

I'm not sure we want to spread special cases across the main test suite for a community-supported dialect.

I believe it is legitimate for tests to also consider community dialects. Especially if the database is also available in Docker and it is documented in the README. What do you think about that @beikov ?"

@VladoKuruc
Copy link
Contributor Author

VladoKuruc commented Sep 13, 2024

I suggest including community dialect tests in CI build, but configured to allow the build to continue even if these tests fail. This would enable to monitor the evolution of dialect support and detect regressions without jeopardizing the overall CI process.

@VladoKuruc VladoKuruc requested a review from mbellade September 17, 2024 07:29
@mbellade
Copy link
Member

@VladoKuruc as I already said, your PR looks fine but I'm reluctant to special-case community dialects in the core test suite. Also, testing community-supported dialects is up to the user / vendor that maintains them and using CI would consume resources. However, I can bring this up with the team and see what they think of it.

Note: you can just skip the failing tests locally while they are not yet implemented / supported by the dialect you're testing

Copy link
Member

@beikov beikov left a comment

Choose a reason for hiding this comment

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

Other than what I commented, this looks ok to me.

@beikov beikov merged commit e2a1434 into hibernate:main Sep 23, 2024
17 of 19 checks passed
@VladoKuruc VladoKuruc deleted the informix_tests branch September 23, 2024 12:44
beikov pushed a commit to beikov/hibernate-orm that referenced this pull request Dec 2, 2024
beikov pushed a commit to beikov/hibernate-orm that referenced this pull request Dec 3, 2024
beikov pushed a commit to beikov/hibernate-orm that referenced this pull request Dec 3, 2024
dreab8 pushed a commit to beikov/hibernate-orm that referenced this pull request Dec 17, 2024
beikov pushed a commit that referenced this pull request Dec 18, 2024
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.

3 participants