-
Notifications
You must be signed in to change notification settings - Fork 125
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
Add an empty database spec #243
Conversation
ea44516
to
b234145
Compare
@@ -0,0 +1,20 @@ | |||
describe "Empty Database" do | |||
let(:connection) { ApplicationRecord.connection } |
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 know we use ActiveRecord::Base
in the actual migrations. Should we use it here too?
@@ -0,0 +1,11 @@ | |||
describe "Empty Database" do | |||
it "after migrated remains empty" do | |||
counts = ApplicationRecord.connection.tables.each_with_object([]) do |t, array| |
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.
Should we use ActiveRecord::Base.connection
here? I know we use it in the migrations, but wasn't sure if we should in the specs as well.
counts = ApplicationRecord.connection.tables.each_with_object([]) do |t, array| | ||
next if ManageIQ::Schema::SYSTEM_TABLES.include?(t) | ||
count = ApplicationRecord.connection.execute("SELECT COUNT(*) FROM #{t};").getvalue(0, 0) | ||
array << "#{t}: #{count}" if count.positive? |
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.
What about negative row counts 😱
Only the schema itself should change
b234145
to
db33c1b
Compare
Checked commits bdunne/manageiq-schema@37a42d2~...db33c1b with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
An empty database (migrated form 0) should remain empty, only the schema itself should change.
Adding this as an automated code review to address #4 (comment)