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
BOOLEAN_ALLOWS_BOOLEAN_MESSAGE warning is invalid #761
Comments
You're right, and I didn't think about that. I'm not sure when I'll be able to get to this, but I'll leave it open until then. |
I don't mind finding the time to get a PR together as well, happy to help!
|
+1 |
1 similar comment
+1 |
I think we should probably remove this warning in the following cases:
The Rails documentation has this to say under
This is technically true, but it will have no effect if either of the conditions above are false. |
Hey folks. In an effort to lighten our load as maintainers and be able to serve you better in the future, the shoulda-matchers team is working on cleaning out the cobwebs in this repo by pruning the backlog. As there are few of us, there are a lot of items that will simply never earn our attention in a reasonable time frame, and rather than giving you an empty promise, we think it makes more sense to focus on more recent issues. That means, unfortunately, that we must close this issue. Don't take this the wrong way: our aim is not to diminish the effort people have made or dismiss problems that have been raised. If you feel that we should reopen this issue, then please let us know so that we can reprioritize it. Thanks! |
I'd love to see this come back. I've hit the same issue where a pure ActiveModel (i.e. non-persisted model class) sets this validation. We'd love to clean up our test suite output to not complain about this valid (in our case and others) implementation. I'd be happy to open up a PR! Please let me know, and thank you for your consideration 🙏 |
@cbortz Yes, please open a PR! |
@mcmire Will do! Would y'all prefer this to be:
I will go ahead and get started on something today regardless. But if there are any preferences or requirements, please let me know! 🙏 |
@cbortz I think it should be based on class ancestry. I doubt that most people use the Attributes API in ActiveModel models, so it is very rare that boolean values will be converted in such a way that the error message indicates. I'll repeat what I said up earlier:
|
Excellent. Thank you for clarifying. |
I have a custom
ActiveModel::Model
in which I'm validating true/false inclusion:This results in the
BOOLEAN_ALLOWS_BOOLEAN_MESSAGE
warning being output during my tests run:The problem with this is that the warning is only valid for ActiveRecord, Mongoid, or other ORMs that translate values to true or false. If just using an ActiveModel or other bare-bones model, this is not true.
This can be demonstrated by simply adding an
allow_value
matcher to the suite:This warning should go away, or it should have an option to disable it since it's incorrectly mucking up my test output.
The text was updated successfully, but these errors were encountered: