remove most of test authentication stubbing #17575
Conversation
kbrock
changed the title
spec/support/auth_helper.rb
Outdated
| User.current_user = user | ||
| puts "WARNING: double stubbing user - only use login_as or stub_user once" if user != User.current_user |
| allow(controller).to receive(:current_user).and_return(user) | ||
| allow(User).to receive(:current_user).and_return(user) | ||
| # Stubs the user in context for a controller | ||
| # @param features (:all|:none|Array<>,String,Symbol) features for a user |
There was a problem hiding this comment.
when specific features are chosen, how are they passed...as Strings? I see Array of symbol, but I would have expected String identifiers.
That being said, there already is an "everything" product feature, so we should deprecate :all, and none can be handled by passing an empty Array, so we should probably deprecate :none as well. Then there are no "special" feature names, only real ones.
There was a problem hiding this comment.
thnx - array of strings or symbols work
There was a problem hiding this comment.
a) I would like to s/all/everything in a future PR. it is in many repos.
b) :none seems to work very well. It creates all the joining models. The user ends up with feature(s), just features that aren't ones that affect us
|
This looks really nice. Much cleaner. |
|
Agreed, looks great, thanks :) |
kbrock
force-pushed
the
authentication_helper_stubless
branch
from
72e4160 to
5b77924
Compare
|
Some comments on commit kbrock@5b77924 spec/support/auth_helper.rb
|
|
Checked commit kbrock@5b77924 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
If we have fewer stubs, then we are testing more code and we are encouraged to pay attention to roles/miq_product_features and authorization.
I can see the argument that this makes it more or less stable.
So I'm putting it out there and seeing what others thing.
I just ran master classic-ui and all tests passed locally
@himdel - thoughts?