Skip to content
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

Clean up AuthenticationWorkflow spec helper #5828

Merged
merged 21 commits into from Aug 4, 2020

Conversation

luisramos0
Copy link
Contributor

@luisramos0 luisramos0 commented Jul 27, 2020

What? Why?

Originated from #5827 I did a few things here:

  • rename AuthenticationWorkflow to AuthenticationHelper
  • delete create_enterprise_user and use the existing user factory: create(:user)
  • rename the methods in ControllerHelper to be more specific, they are really setting up mocks, not logging in
  • rename the methods quick_login_ to simply login_ as they are the main methods that should be used to login
  • added method login_as_admin_and_visit and used it in quite a few places, this not only removes some lines of code from the specs but most importantly removes quite a lot of useless page loads of the admin dashboard page in login_to_admin_as

What should we test?

This is only changing specs, a green build is enough.

Release notes

Changelog Category: Changed
Simplified the code and improved speed of specs.

@luisramos0 luisramos0 force-pushed the ent_user branch 2 times, most recently from 34e8a00 to b111b5b Compare July 27, 2020 13:17
@luisramos0 luisramos0 changed the title Ent user Clean up AuthenticationWorkflow spec helper Jul 27, 2020
@luisramos0 luisramos0 self-assigned this Jul 27, 2020
@luisramos0 luisramos0 force-pushed the ent_user branch 2 times, most recently from 66f08ab to 520c6b7 Compare July 27, 2020 13:56
@luisramos0
Copy link
Contributor Author

nice, thanks Pau. Can you approve the PR?

Copy link
Contributor

@sauloperez sauloperez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ups, sorry

# # Equivalent to being in spec/controllers
# end
config.infer_spec_type_from_file_location!
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just noticed this. This block shouldn't be here at all but in spec/spec_helper.rb. I'll patch this myself and it'll be ready to go. The longer we wait the more chances of this PR becoming stale.

Spec files individually include the module and we specify the type of
spec in each RSpec's describe so none of this settings are needed. They
are just Spree's legacy I bet.
@sauloperez sauloperez self-assigned this Aug 4, 2020
Apparently, although we tend to add the type of spec file some RSpec
methods are not working without it. We're getting:

```
NoMethodError:
  undefined method `helper' for RSpec::ExampleGroups::SpreeSharedOrderDetailsHtmlHaml:Class
```

```
NameError:
   undefined local variable or method `controller' for #<RSpec::ExampleGroups::SpreeAdminUsersController::AuthorizeAdmin:0x00007fa8b32addf8>
 # ./spec/controllers/spree/admin/users_controller_spec.rb:10:in `block (3 levels) in <top (required)>'
```

It needs more investigation but another day.
@sauloperez
Copy link
Contributor

Here comes my hope as a new commit :trollface:

@sauloperez
Copy link
Contributor

Green light! Merging!

@sauloperez sauloperez merged commit 6e1dd78 into openfoodfoundation:master Aug 4, 2020
@luisramos0 luisramos0 deleted the ent_user branch August 4, 2020 10:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants