Shoulda-matchers: is_expected.to validate_inclusion_of(:role).in_array(%w(user blocked) raises ArgumentError

[majioa]

I have the following matcher:

it { is_expected.to validate_inclusion_of(:role).in_array(%w(user blocked))

when I run it to test I get strange error:

ArgumentError:
  '123456789' is not a valid role

The model code is:

enum role: [:user, :blocked]

validates :role, inclusion: { in: %w(user blocked) }

What is wrong here?

[tejasbubane]

If I am not wrong, that error is thrown by Rails enum when shoulda matchers tries to assign an arbitrary value to check for inclusion (expecting it to be false).

Enums will make sure any value apart from the enum values will not be assigned. Hence you don't need to have the inclusion validation for enum, rails implicilty does that for you.

What you need to do is check if an enum is defined. You can use define_enum_for: https://github.com/thoughtbot/shoulda-matchers/blob/master/lib/shoulda/matchers/active_record/define_enum_for_matcher.rb#L36 which also allows checking values.

it { is_expected.to define_enum_for(:role).with(%w(user blocked)) }

[majioa]

thanx

[gkop]

define_enum_for doesn't permit checking for conditional validations on the allowed values.

@tejasbubane any reason we couldn't catch the ArgumentError in shoulda-matchers and treat it as a positive assertion?

[tejasbubane]

@gkop Not sure I get what you mean by conditional validations. Could you share and example?

any reason we couldn't catch the ArgumentError in shoulda-matchers and treat it as a positive assertion?

I think doing this is not a good idea since validate_inclusion_ofis a generic matcher and not specific to enums. But this is just my opinion.

Also, I came across this discussion in rails issues on using validations with enums: https://github.com/rails/rails/issues/13971#issuecomment-34530922. Might be relevant in this case.

[gkop]

Thanks for the quick reply! ActiveRecord validations support the options :if and :unless to scope validations to certain conditions. Generally, shoulda-matchers has good support for testing validations under different conditions. But in this case of enums shoulda-matchers does not work.

We can support enums better in shoulda-matchers...

Ideas

1) We could suppress the assignment of an arbitrary value (IE "123456789") to an attribute when we detect that the attribute is an enum during assignment validations like those performed by validate_inclusion_of. We could even in this case print a warning advising to use define_enum_for instead of or in addition to assignment validations for best enum coverage.

or

2) We could continue doing the assignments of an arbitrary value, but detect if the attribute is an enum, and in that case look for the ArgumentError and consider it a positive assertion.

[tejasbubane]

We could even in this case print a warning advising to use define_enum_for instead of validates_inclusion_of

This sounds like a good idea. @mcmire What do you think?

[mcmire]

I think that's a good idea, although it seems like the real issue here is that people are adding an inclusion validation to their model as extra support behind an enum. So if I were to add a warning message, it would be fairly prescriptive:

We've noticed that you're using validate_inclusion_of to test an enum attribute. Enums are, by definition, protected so that they only allow specific values. In other words, ActiveRecord does this work for you already. So you don't need an inclusion validation in your model, and you don't need to use validate_inclusion_of, either.

Would anyone be opposed to me adding a message like this?

[gkop]

To be clear, my primary interest is gaining the ability to use validate_inclusion_of on enums so I can test my validation that only subsets of the enum values are allowed under different conditions. The warning message strikes me as a good idea - since I am intending to use validate_inclusion_of on enums, it'd be nice to have an option to suppress the message though ;)

[mcmire]

@gkop Okay, gotcha. Well, that's why I asked first :)

The ArgumentError that was mentioned above is definitely coming from Rails: https://github.com/rails/rails/blob/master/activerecord/lib/active_record/enum.rb#L139

So, in that case, I think the only course of action here would be to modify define_enum_for to rescue the ArgumentError, and if the error message matches /is not a valid #{attribute}/ then we swallow the error and move on.