Skip to content
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

fix: migrator-integration-test tests not running #191 #192

Merged
merged 14 commits into from
May 21, 2024

Conversation

Roiocam
Copy link
Member

@Roiocam Roiocam commented May 15, 2024

Motivation

Resolves: #191

Resolves in step by step

  • investigation Migrator integration test compile issue.
  • investigation package name is issue or not.
  • investigation integration/testOnly will be an issue or not.

compile issue

migratorIntegration should be depend on migrator

package name issue & sbt task issue

  • sbt ++2.13 "clean; integration/testOnly org.apache.pekko.persistence.jdbc.integration.Postgres*"
  • sbt ++2.13 "clean; testOnly org.apache.pekko.persistence.jdbc.integration.Postgres*": doesn't run anything
  • sbt ++2.13 "clean; migratorIntegration/testOnly org.apache.pekko.persistence.jdbc.integration.Postgres*": doesn't run anything

both are unable to run PostgresJournalMigratorTest, both the package name and sbt tasks integration/testOnly have been verified to cause issues.

verify the solution

  • sbt ++2.13 "integration/testOnly org.apache.pekko.persistence.jdbc.integration.Postgres*; migratorIntegration/testOnly org.apache.pekko.persistence.jdbc.migrator.integration.*" works, but looks like very long. ✅
  • sbt ++2.13 "integration/testOnly; migratorIntegration/testOnly org.apache.pekko.persistence.jdbc.integration.Postgres*" can not filter other RDBMB.

@pjfanning
Copy link
Contributor

As far as I can see, the main issue is that the yml files for the GitHub workflows run integation/testOnly ...

migratorIntegration is not included in the root project so I'm not even sure you can run the tests locally.

build.sbt Show resolved Hide resolved
@Roiocam
Copy link
Member Author

Roiocam commented May 15, 2024

Seems like JournalMigratorTest is incorrect in SQLServer and Oracle:

in SQLServer:

JournalMigrator(SlickDatabase.profile(config, "slick")).migrate().futureValue shouldBe Done

will complain:

com.microsoft.sqlserver.jdbc.SQLServerException, with message: Cannot insert explicit value for identity column in table 'event_journal' when IDENTITY_INSERT is set to OFF...

@pjfanning
Copy link
Contributor

@Roiocam I want to release v1.1.0-M1 of this module within the next week or 2. Would it be possible just to enable the tests for the dbs where this currently works and for us to come back to the broken ones after the v1.1.0-M1 release?

@Roiocam
Copy link
Member Author

Roiocam commented May 19, 2024

@pjfanning This PR is ready for review.

@Roiocam Roiocam requested a review from pjfanning May 19, 2024 13:46
Copy link
Contributor

@pjfanning pjfanning left a comment

Choose a reason for hiding this comment

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

lgtm

@Roiocam
Copy link
Member Author

Roiocam commented May 19, 2024

@pjfanning Thanks for the review, I also fix SQLServer tests, can you take a look again?

@pjfanning
Copy link
Contributor

@pjfanning Thanks for the review, I also fix SQLServer tests, can you take a look again?

thanks @Roiocam that sqlserver fix looks good.

@Roiocam Roiocam merged commit ed3dafc into apache:main May 21, 2024
23 checks passed
@Roiocam Roiocam deleted the fix-it branch May 21, 2024 02:19
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.

migrator-integration-test tests are not run
3 participants