shoulda-matchers
shoulda-matchers copied to clipboard
sql_type check in have_db_column
Would you guys be open to a PR to add a method into the have_db_column chain that checks the sql_type of the column? Currently there is only one for type which I think is insufficient when dealing with certain column types.
For instance, say you are storing guids in a char(36) column, rails will interpret that as :string. In this case it is quite important that the column is actually a mysql char(36) instead.
irb(main):008:0> MyModel.columns.find { |c| c.name == 'some_guid_column' }.type
=> :string
irb(main):009:0> MyModel.columns.find { |c| c.name == 'some_guid_column' }.sql_type
=> "char(36)"
irb(main):010:0>
I was thinking to just add an additional method into have_db_column so you can specify something like:
it { is_expected.to have_db_column(:some_guid_column).of_sql_type('char(36)') }
I'm happy to do the PR if you guys are open to it.
On the surface it seems like a small change, but I'm curious, are you worried that Rails will do the wrong thing here and not assign the correct SQL type? I have been increasingly questioning the value of have_db_column as it seems to be testing configuration more than logic. But I first want to make sure there is not something I'm missing about how the matcher is getting used.
I suppose it is more testing configuration than logic when you think hard about it, but given that Rails models are so inherently tied to the database it seems sensible to me to have a buffer between changes.
When you have migrations being picked up from so many different engines like we do it makes sense to have the engine that requires the presence of a particular field also validate that the field exists when the tests across all engines are run (makes sure another engine doesn't have conflicting migrations etc). Especially when those migrations use actual sql types rather than the rails built-in string with limit.
It's also pretty good documentation for the model itself that saves having to dive through schema.rb files.
I guess it may make less sense in the typical rails app setup rather than an application split into 20 or so engines? It's up to you, I wrote a custom matcher for our stuff that did this - I just thought it'd be nice to upstream it if other people would also like to validate db schema in the same way.
Ah, engines, okay. I can see how that could end up creating potentially confusing situations. And my second question was going to be about whether you are using t.column :some_guid_column, 'char(36)' or t.string :some_guid_column, :string, limit: 36, but looks like you already answered that question.
Okay, I can see the use case behind this. Send a PR over and we will be happy to merge this. :)