From 9de7645c5b83705af6f226bdf93bd96a3b18060c Mon Sep 17 00:00:00 2001 From: Sean Collins Date: Thu, 18 Jul 2024 16:03:09 -0600 Subject: [PATCH 1/7] Use docker port, provide password, add note to README --- README.md | 2 ++ docker-compose.yml | 1 - spec/support/postgres.rb | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index f93827b5..cc432c3a 100644 --- a/README.md +++ b/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). diff --git a/docker-compose.yml b/docker-compose.yml index f3d87596..160b6501 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,4 +1,3 @@ -version: "2" services: postgres: image: postgres:latest diff --git a/spec/support/postgres.rb b/spec/support/postgres.rb index 450c418a..9e65c041 100644 --- a/spec/support/postgres.rb +++ b/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| From 14cd05c73434126cda2f68ee9dcfbf221f995eb8 Mon Sep 17 00:00:00 2001 From: Sean Collins Date: Fri, 19 Jul 2024 11:47:52 -0600 Subject: [PATCH 2/7] Improve warning for missing config/db/ folder --- lib/hanami/cli/commands/app/db/command.rb | 3 ++- spec/unit/hanami/cli/commands/app/db/migrate_spec.rb | 6 ++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/hanami/cli/commands/app/db/command.rb b/lib/hanami/cli/commands/app/db/command.rb index 83d0e4a0..e241e408 100644 --- a/lib/hanami/cli/commands/app/db/command.rb +++ b/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 diff --git a/spec/unit/hanami/cli/commands/app/db/migrate_spec.rb b/spec/unit/hanami/cli/commands/app/db/migrate_spec.rb index ce59c138..4565560a 100644 --- a/spec/unit/hanami/cli/commands/app/db/migrate_spec.rb +++ b/spec/unit/hanami/cli/commands/app/db/migrate_spec.rb @@ -323,7 +323,7 @@ class Comments < Hanami::DB::Relation end end - context "no db config dir" do + context "no db/config dir" do def before_prepare write "app/relations/.keep", "" end @@ -335,7 +335,9 @@ def before_prepare 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( + "WARNING: Database db/app.sqlite3 expects the folder config/db/ to exist but it does not." + ) expect(output).not_to include "migrated" end end From 6d40b63922aa3cfe68e7866e9d4f5cfa37acb651 Mon Sep 17 00:00:00 2001 From: Sean Collins Date: Fri, 19 Jul 2024 12:01:09 -0600 Subject: [PATCH 3/7] Add warning when config/db/migrate is missing --- lib/hanami/cli/commands/app/db/migrate.rb | 15 +++++++++++- .../cli/commands/app/db/migrate_spec.rb | 23 ++++++++++++++++++- 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/lib/hanami/cli/commands/app/db/migrate.rb b/lib/hanami/cli/commands/app/db/migrate.rb index 9088f074..4b1cc4c2 100644 --- a/lib/hanami/cli/commands/app/db/migrate.rb +++ b/lib/hanami/cli/commands/app/db/migrate.rb @@ -15,7 +15,11 @@ 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 database.migrations_dir? + migrate_database(database, target: target) + else + warn_on_missing_migrations_dir(database) + end end run_command(Structure::Dump, app: app, slice: slice, command_exit: command_exit) if dump @@ -40,6 +44,15 @@ def migrate_database(database, target:) def migrations?(database) database.migrations_dir? && database.sequel_migrator.files.any? end + + def warn_on_missing_migrations_dir(database) + relative_path = database.slice.root.relative_path_from(database.slice.app.root).join("config", "db", "migrate").to_s + out.puts <<~STR + WARNING: Database #{database.name} expects migrations to be located within #{relative_path}/ but that folder does not exist. + + No database migrations can be run for this database. + STR + end end end end diff --git a/spec/unit/hanami/cli/commands/app/db/migrate_spec.rb b/spec/unit/hanami/cli/commands/app/db/migrate_spec.rb index 4565560a..da2d12e6 100644 --- a/spec/unit/hanami/cli/commands/app/db/migrate_spec.rb +++ b/spec/unit/hanami/cli/commands/app/db/migrate_spec.rb @@ -323,7 +323,7 @@ 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 @@ -341,4 +341,25 @@ def before_prepare 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 end From 47609d3ca455c59b03820230479da098233bbdec Mon Sep 17 00:00:00 2001 From: Sean Collins Date: Fri, 19 Jul 2024 12:19:20 -0600 Subject: [PATCH 4/7] Add warning for empty config/db/migrate folder --- lib/hanami/cli/commands/app/db/migrate.rb | 29 +++++++++----- .../cli/commands/app/db/migrate_spec.rb | 39 +++++++++++-------- 2 files changed, 43 insertions(+), 25 deletions(-) diff --git a/lib/hanami/cli/commands/app/db/migrate.rb b/lib/hanami/cli/commands/app/db/migrate.rb index 4b1cc4c2..df61ac1d 100644 --- a/lib/hanami/cli/commands/app/db/migrate.rb +++ b/lib/hanami/cli/commands/app/db/migrate.rb @@ -15,10 +15,12 @@ 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| - if database.migrations_dir? - migrate_database(database, target: target) - else + 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 @@ -28,21 +30,21 @@ 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) @@ -53,6 +55,15 @@ def warn_on_missing_migrations_dir(database) No database migrations can be run for this database. STR end + + def warn_on_empty_migrations_dir(database) + relative_path = database.slice.root.relative_path_from(database.slice.app.root).join("config", "db", "migrate").to_s + out.puts <<~STR + WARNING: Database #{database.name} has the correct migrations folder #{relative_path}/ but that folder is empty. + + No database migrations can be run for this database. + STR + end end end end diff --git a/spec/unit/hanami/cli/commands/app/db/migrate_spec.rb b/spec/unit/hanami/cli/commands/app/db/migrate_spec.rb index da2d12e6..9d6e2bdd 100644 --- a/spec/unit/hanami/cli/commands/app/db/migrate_spec.rb +++ b/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 @@ -362,4 +346,27 @@ def before_prepare 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 include( + "WARNING: Database db/app.sqlite3 has the correct migrations folder config/db/migrate/ but that folder is empty." + ) + expect(output).to include("No database migrations can be run for this database.") + expect(output).not_to include "migrated" + end + end end From 1cb693505197708fc306a558fade91dfbd1fdefb Mon Sep 17 00:00:00 2001 From: Sean Collins Date: Fri, 19 Jul 2024 12:22:53 -0600 Subject: [PATCH 5/7] Extract helper method --- lib/hanami/cli/commands/app/db/migrate.rb | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/lib/hanami/cli/commands/app/db/migrate.rb b/lib/hanami/cli/commands/app/db/migrate.rb index df61ac1d..196e3db7 100644 --- a/lib/hanami/cli/commands/app/db/migrate.rb +++ b/lib/hanami/cli/commands/app/db/migrate.rb @@ -48,22 +48,28 @@ def no_migrations?(database) end def warn_on_missing_migrations_dir(database) - relative_path = database.slice.root.relative_path_from(database.slice.app.root).join("config", "db", "migrate").to_s out.puts <<~STR - WARNING: Database #{database.name} expects migrations to be located within #{relative_path}/ but that folder does not exist. + 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) - relative_path = database.slice.root.relative_path_from(database.slice.app.root).join("config", "db", "migrate").to_s out.puts <<~STR - WARNING: Database #{database.name} has the correct migrations folder #{relative_path}/ but that folder is empty. + WARNING: Database #{database.name} has the correct migrations folder #{relative_migrations_dir(database)} but that folder is empty. No database migrations can be run for this database. STR end + + def relative_migrations_dir(database) + database + .slice + .root + .relative_path_from(database.slice.app.root) + .join("config", "db", "migrate") + "/" + end end end end From 2fbd548725139073a7681bab4c8c7090bb01380f Mon Sep 17 00:00:00 2001 From: Sean Collins Date: Fri, 19 Jul 2024 12:30:28 -0600 Subject: [PATCH 6/7] Add .to_s --- lib/hanami/cli/commands/app/db/migrate.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/hanami/cli/commands/app/db/migrate.rb b/lib/hanami/cli/commands/app/db/migrate.rb index 196e3db7..b6e5d0e4 100644 --- a/lib/hanami/cli/commands/app/db/migrate.rb +++ b/lib/hanami/cli/commands/app/db/migrate.rb @@ -68,7 +68,8 @@ def relative_migrations_dir(database) .slice .root .relative_path_from(database.slice.app.root) - .join("config", "db", "migrate") + "/" + .join("config", "db", "migrate") + .to_s + "/" end end end From 2156778efc371d4c873ee98d569cf1df6eb7369d Mon Sep 17 00:00:00 2001 From: Sean Collins Date: Thu, 25 Jul 2024 23:57:07 -0600 Subject: [PATCH 7/7] Change warning to NOTE --- lib/hanami/cli/commands/app/db/migrate.rb | 4 +--- spec/unit/hanami/cli/commands/app/db/migrate_spec.rb | 3 +-- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/lib/hanami/cli/commands/app/db/migrate.rb b/lib/hanami/cli/commands/app/db/migrate.rb index b6e5d0e4..6743634f 100644 --- a/lib/hanami/cli/commands/app/db/migrate.rb +++ b/lib/hanami/cli/commands/app/db/migrate.rb @@ -57,9 +57,7 @@ def warn_on_missing_migrations_dir(database) def warn_on_empty_migrations_dir(database) out.puts <<~STR - WARNING: Database #{database.name} has the correct migrations folder #{relative_migrations_dir(database)} but that folder is empty. - - No database migrations can be run for this database. + NOTE: Empty database migrations folder (#{relative_migrations_dir(database)}) for #{database.name} STR end diff --git a/spec/unit/hanami/cli/commands/app/db/migrate_spec.rb b/spec/unit/hanami/cli/commands/app/db/migrate_spec.rb index 9d6e2bdd..408de986 100644 --- a/spec/unit/hanami/cli/commands/app/db/migrate_spec.rb +++ b/spec/unit/hanami/cli/commands/app/db/migrate_spec.rb @@ -363,9 +363,8 @@ def before_prepare command.call expect(output).to include( - "WARNING: Database db/app.sqlite3 has the correct migrations folder config/db/migrate/ but that folder is empty." + "NOTE: Empty database migrations folder (config/db/migrate/) for db/app.sqlite3" ) - expect(output).to include("No database migrations can be run for this database.") expect(output).not_to include "migrated" end end