-
Notifications
You must be signed in to change notification settings - Fork 11
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
Update Scala, sbt and dependencies #46
base: master
Are you sure you want to change the base?
Conversation
9e51f7c
to
27ca74a
Compare
Update dependencies Update Scala Update munit 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.
Looks great, thanks for taking this on. Left a few comments.
And I appreciate the offer to add a CI. Since they're just a few test dependencies I'm fine doing this manually for now.
"com.dimafeng" %% "testcontainers-scala-munit" % testcontainersVersion % Test, | ||
"com.dimafeng" %% "testcontainers-scala-postgresql" % testcontainersVersion % Test, | ||
"org.postgresql" % "postgresql" % "42.6.0" % Test, | ||
"org.postgresql" % "postgresql" % "42.7.4" % Test, |
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 you also update the docker images to the latest LTS release? For example in PgTests:464
val pgContainer = ForAllContainerFixture(
PostgreSQLContainer
.Def(dockerImageName = DockerImageName.parse("postgres:15.2"))
.createContainer()
)
We can use the 17.0 image instead.
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.
Done 🙂
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.
Thanks, please also update the docker images for the other Dbs as well:
- ClickHouseTests:393 Version: 24.3.12.75
- MySqlTests:395 Version: 8.4.2
- OracleTests:391 Switch to use https://hub.docker.com/r/gvenzl/oracle-free and version 23.5-faststart (or just 23.5 if that one doesn't work)
- Also, PgCodecTests:157 in magnum-pg module to 17.0
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.
@AugustNagro Done :)
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.
Thanks, getting
java.lang.IllegalStateException: Failed to verify that image 'gvenzl/oracle-free:23.5-faststart' is a compatible substitute for 'gvenzl/oracle-xe'. This generally means that you are trying to use an image that Testcontainers has not been designed to use. If this is deliberate, and if you are confident that the image is compatible, you should declare compatibility in code using the `asCompatibleSubstituteFor` method. For example:
DockerImageName myImage = DockerImageName.parse("gvenzl/oracle-free:23.5-faststart").asCompatibleSubstituteFor("gvenzl/oracle-xe");
With the new Oracle image.. would you mind reverting to the earlier gvenzl/oracle-xe? It's not up-to-date with the latest release but there's no rush if test-containers doesn't support it yet.
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.
MySQL tests are failing for mysql:8.4.3, with
Could not create/start container
Looking at the docker logs, it exits because of a removed option:
unknown option '--skip-host-cache'
This was reported in testcontainers-java and fixed in 1.20.2
However, testcontainers-scala depends on v 1.19.x .
I've filed an issue to upgrade: testcontainers/testcontainers-scala#368
For now, lets just revert the MySQL image upgrade as well.
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.
@AugustNagro Can we merge #53 maybe, please? That will help me detect these stuff
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 reviewed #53 and left a few comments.
Just fyi it's very easy to run the tests locally. You just need to install docker (docker desktop or docker engine), then run sbt test
.
Review: Update to mysql-connector-j
Hum, I'd prefer if we add a minimal CI running tests. |
Yeah CI to run the tests would be very appreciated. To have something like scala-steward auto-updating the dependencies would be more of a PITA I think. Since I'm not sure scala-steward would be able to update the docker images to the latest LTS releases. |
Let's forget about scala-steward if you don't like it. Let's just add a minimal CI which runs the tests. I'll make another PR for that if that's of for you? |
Perfect, thanks |
Fixes #45
@AugustNagro There's no CI configured? 🤔
Do you want me to configure a minimal Github Actions CI?