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: Database Migration does set current version in DB #98 #103

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

alquerci
Copy link

@alquerci alquerci commented Dec 14, 2023

Start with adding tests, someone else can pick it, to provide a fix.

Depends on #90 to run tests.

Tests failed from PHP 8.0.

fixes #98

@alquerci alquerci force-pushed the fix-database-migration-does-set-current-version-in-db-98 branch from 20059c9 to a29305f Compare December 14, 2023 21:20
@thePanz thePanz force-pushed the fix-database-migration-does-set-current-version-in-db-98 branch 2 times, most recently from da7aca7 to a657313 Compare January 22, 2024 10:36
@alquerci alquerci force-pushed the fix-database-migration-does-set-current-version-in-db-98 branch from a657313 to 270b6a4 Compare April 8, 2024 19:23
@alquerci
Copy link
Author

alquerci commented Apr 8, 2024

With the patch provided on #131, I release this PR.

Sadly, the current CI does not support its execution as missing a MySQL server.

Needs to provide a MySQL server linked through socket.
Or passing the DSN as environment variable.

@alquerci alquerci changed the title [WIP] fix: Database Migration does set current version in DB #98 fix: Database Migration does set current version in DB #98 Apr 8, 2024
@alquerci alquerci force-pushed the fix-database-migration-does-set-current-version-in-db-98 branch from 7827957 to 8cf4b79 Compare April 8, 2024 20:16
@alquerci
Copy link
Author

alquerci commented Apr 8, 2024

To make it easy to configure, I just extracted the DSN configuration from code to environment variable MYSQL_DSN.

As an example, 2dbee52

    environment:
      MYSQL_DSN: 'mysql:unix_socket=/var/run/mysqld/mysql.sock;dbname=test;user=root'

@alquerci alquerci force-pushed the fix-database-migration-does-set-current-version-in-db-98 branch 2 times, most recently from 3c0676d to 5d2f9b5 Compare April 13, 2024 16:25
@alquerci
Copy link
Author

alquerci commented Apr 13, 2024

I found how to add MySQL server on GitHub action.

e9068b2

Now tests passes. 🟢

@alquerci alquerci force-pushed the fix-database-migration-does-set-current-version-in-db-98 branch from 5d2f9b5 to 5045b52 Compare April 13, 2024 16:30
@@ -279,6 +279,7 @@
// Migration Tests
$migration = new GroupTest('Migration Tests', 'migration');
$migration->addTestCase(new Doctrine_Migration_TestCase());
$migration->addTestCase(new Doctrine_Migration_Mysql_TestCase());
Copy link
Member

@thePanz thePanz May 24, 2024

Choose a reason for hiding this comment

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

This test is missing in the PR, is it intentional? @alquerci ?

Copy link
Author

Choose a reason for hiding this comment

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

How is it possible that tests passes?

I will check this mistery in few minutes.

Copy link
Author

Choose a reason for hiding this comment

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

I understood why.

DoctrineTest::autoload() will auto generate test class if it does not exist.
And all *TestCase.php files are ignored by git, see .gitignore


I rebase and add the not committed file.
Then push.

@alquerci alquerci force-pushed the fix-database-migration-does-set-current-version-in-db-98 branch 2 times, most recently from 5e8c299 to ad4b0f2 Compare May 24, 2024 17:38
@alquerci
Copy link
Author

alquerci commented May 24, 2024

@thePanz After the fix of MYSQL_DSN on 4a6c603

Tests passes.

@alquerci alquerci force-pushed the fix-database-migration-does-set-current-version-in-db-98 branch from ad4b0f2 to ac331ed Compare July 3, 2024 19:26
@alquerci
Copy link
Author

alquerci commented Jul 3, 2024

Rebased onto master to fix conflicts with refacto #138

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.

Database Migration does set current version in DB
2 participants