Improve db/migrations misconfigurations

README.md

@@ -34,6 +34,8 @@ After checking out the repo, run `bin/setup` to install dependencies. Then, run
 
 To install this gem onto your local machine, run `bundle exec rake install`. To release a new version, update the version number in `version.rb`, and then run `bundle exec rake release`, which will create a git tag for the version, push git commits and the created tag, and push the `.gem` file to [rubygems.org](https://rubygems.org).
 
+In order to run all of the tests, you should run `docker compose up` separately, to run a `postgres` server.
+
 ## Contributing
 
 Bug reports and pull requests are welcome on GitHub at https://github.com/hanami/cli. This project is intended to be a safe, welcoming space for collaboration, and contributors are expected to adhere to the [code of conduct](https://github.com/hanami/cli/blob/main/CODE_OF_CONDUCT.md).

docker-compose.yml

@@ -1,4 +1,3 @@
-version: "2"
 services:
   postgres:
     image: postgres:latest

lib/hanami/cli/commands/app/db/command.rb

@@ -102,8 +102,9 @@ def warn_on_misconfigured_database(database, slices)
 
                 STR
               elsif slices.length < 1
+                relative_path = database.slice.root.relative_path_from(database.slice.app.root).join("config", "db").to_s
                 out.puts <<~STR
-                  WARNING: Database #{database.name} has no config/db/ directory.
+                  WARNING: Database #{database.name} expects the folder #{relative_path}/ to exist but it does not.
 
                 STR
               end

lib/hanami/cli/commands/app/db/migrate.rb

@@ -15,7 +15,13 @@ class Migrate < DB::Command
 
             def call(target: nil, app: false, slice: nil, dump: true, command_exit: method(:exit), **)
               databases(app: app, slice: slice).each do |database|
-                migrate_database(database, target: target)
+                if migrations_dir_missing?(database)
+                  warn_on_missing_migrations_dir(database)
+                elsif no_migrations?(database)
+                  warn_on_empty_migrations_dir(database)
+                else
+                  migrate_database(database, target: target)
+                end
               end
 
               run_command(Structure::Dump, app: app, slice: slice, command_exit: command_exit) if dump
@@ -24,21 +30,44 @@ def call(target: nil, app: false, slice: nil, dump: true, command_exit: method(:
             private
 
             def migrate_database(database, target:)
-              return true unless migrations?(database)
-
               measure "database #{database.name} migrated" do
                 if target
                   database.run_migrations(target: Integer(target))
                 else
                   database.run_migrations
                 end
-
-                true
               end
             end
 
-            def migrations?(database)
-              database.migrations_dir? && database.sequel_migrator.files.any?
+            def migrations_dir_missing?(database)
+              !database.migrations_dir?
+            end
+
+            def no_migrations?(database)
+              database.sequel_migrator.files.empty?
+            end
+
+            def warn_on_missing_migrations_dir(database)
+              out.puts <<~STR
+                WARNING: Database #{database.name} expects migrations to be located within #{relative_migrations_dir(database)} but that folder does not exist.
+
+                No database migrations can be run for this database.
+              STR
+            end
+
+            def warn_on_empty_migrations_dir(database)
+              out.puts <<~STR
+                NOTE: Empty database migrations folder (#{relative_migrations_dir(database)}) for #{database.name}
+              STR
+            end
+
+            def relative_migrations_dir(database)
+              database
+                .slice
+                .root
+                .relative_path_from(database.slice.app.root)
+                .join("config", "db", "migrate")
+                .to_s + "/"
             end
           end
         end

spec/support/postgres.rb

@@ -5,7 +5,7 @@
 
 # Default to a URL that should work with postgres as installed by asdf/mise.
 POSTGRES_BASE_DB_NAME = "hanami_cli_test"
-POSTGRES_BASE_URL = ENV.fetch("POSTGRES_BASE_URL", "postgres://postgres@localhost:5432/#{POSTGRES_BASE_DB_NAME}")
+POSTGRES_BASE_URL = ENV.fetch("POSTGRES_BASE_URL", "postgres://postgres:postgres@localhost:5433/#{POSTGRES_BASE_DB_NAME}")
 POSTGRES_BASE_URI = URI(POSTGRES_BASE_URL)
 
 RSpec.configure do |config|

spec/unit/hanami/cli/commands/app/db/migrate_spec.rb

@@ -240,22 +240,6 @@ class Comments < Hanami::DB::Relation
     end
   end
 
-  context "no migration files" do
-    def before_prepare
-      write "config/db/migrate/.keep", ""
-    end
-
-    before do
-      ENV["DATABASE_URL"] = "sqlite://db/app.sqlite3"
-      db_create
-    end
-
-    it "does nothing" do
-      command.call
-      expect(output).to be_empty
-    end
-  end
-
   context "multiple dbs with identical database_url" do
     def before_prepare
       write "slices/admin/config/db/migrate/20240602201330_create_posts.rb", <<~RUBY
@@ -323,19 +307,64 @@ class Comments < Hanami::DB::Relation
     end
   end
 
-  context "no db config dir" do
+  context "no db/config/" do
+    def before_prepare
+      write "app/relations/.keep", ""
+    end
+
+    before do
+      ENV["DATABASE_URL"] = "sqlite://db/app.sqlite3"
+    end
+
+    it "prints a warning, and does not migrate the database" do
+      command.call
+
+      expect(output).to include(
+        "WARNING: Database db/app.sqlite3 expects the folder config/db/ to exist but it does not."
+      )
+      expect(output).not_to include "migrated"
+    end
+  end
+
+  context "no db/config/migrate/" do
+    def before_prepare
+      write "app/relations/.keep", ""
+      write "config/db/.keep", ""
+    end
+
+    before do
+      ENV["DATABASE_URL"] = "sqlite://db/app.sqlite3"
+    end
+
+    it "prints a warning, and does not migrate the database" do
+      command.call
+
+      expect(output).to include(
+        "WARNING: Database db/app.sqlite3 expects migrations to be located within config/db/migrate/ but that folder does not exist."
+      )
+      expect(output).to include("No database migrations can be run for this database.")
+      expect(output).not_to include "migrated"
+    end
+  end
+
+  context "empty db/config/migrate/" do
     def before_prepare
       write "app/relations/.keep", ""
+      write "config/db/.keep", ""
+      write "config/db/migrate/.keep", ""
     end
 
     before do
       ENV["DATABASE_URL"] = "sqlite://db/app.sqlite3"
+      db_create
     end
 
     it "prints a warning, and does not migrate the database" do
       command.call
 
-      expect(output).to match %{WARNING:.+no config/db/ directory.}
+      expect(output).to include(
+        "NOTE: Empty database migrations folder (config/db/migrate/) for db/app.sqlite3"
+      )
       expect(output).not_to include "migrated"
     end
   end