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

Are you open to include rspec_matchers? #755

Closed
stoivo opened this issue Jul 7, 2023 · 18 comments
Closed

Are you open to include rspec_matchers? #755

stoivo opened this issue Jul 7, 2023 · 18 comments

Comments

@stoivo
Copy link

stoivo commented Jul 7, 2023

I have used have_http_status from rspec-rails for a while an like it a lot. Would you be okay with including a custom rspec marcher?

I would like to implement something which looks like have_http_status. I was thinking of naming it have_httprb_status

@stoivo
Copy link
Author

stoivo commented Jul 7, 2023

One alternative is to create a separate gem which has these marchers but I think it would be nice to have it here

@tarcieri
Copy link
Member

A separate gem with the matchers seems good to me. @ixti what do you think?

@ixti
Copy link
Member

ixti commented Jul 14, 2023

Yeah, I believe separate gem is the best approach.

@ixti
Copy link
Member

ixti commented Jul 14, 2023

Just looked at Rails matcher impementation, we can actually provide 2 methods on response to make it compatible with have_http_status:

alias status_code code

def response_headers
  headers.to_h
end

@stoivo
Copy link
Author

stoivo commented Jul 22, 2023

That would be an options but we don't know if the user are using or want to include rspec-rails. In this project I do but i see myself using rails less and less when hanami gets better

@stoivo
Copy link
Author

stoivo commented Jul 26, 2023

If I create a repo with http_rb_rspec gem, with matchers. Would it be okey for it to live in httprb so you could do stuff with it if I'm not available. I can maintain it and release it but I would be nice it could live here to it's yours.

@ixti
Copy link
Member

ixti commented Jul 27, 2023 via email

@ixti
Copy link
Member

ixti commented Jul 27, 2023

I'll create a repo and grant you access today.

@stoivo
Copy link
Author

stoivo commented Jul 31, 2023

I have the matcher and the gem ready ready. I can't find the invite did you create it?

@stoivo
Copy link
Author

stoivo commented Aug 1, 2023

I created a repo just to show what I did https://github.com/stoivo/httprb_rspec

@ixti
Copy link
Member

ixti commented Aug 1, 2023

Sorry. Was off the grid a bit. Created http-rspec repo and invited you to it.

A couple of thoughts on that one:

  1. I think a better name would be http-rspec (so that it's http gem's name + rspec)
  2. Default require should be: require "http/rspec"
  3. I believe we can use have_http_status as matcher name, or, to avoid any name conflicts (and deal with conditional proxying) we can have matcher: be_an_http_gem_response.with(status: ...)

@stoivo
Copy link
Author

stoivo commented Aug 2, 2023

  1. Oh, thought I check I thought it was taken. I agree that we should use this name instead.
  2. Ok. will fix
  3. rspec-rails adds have_http_status to all scoped so we will have to deal with conflict we we use that name. https://github.com/rspec/rspec-rails/blob/71a5751760f64ec86c2c51545c9894abee867d6f/lib/rspec/rails/configuration.rb#L55-L55. I do think be_an_http_gem_response.with(status: ... ) is a bit long, but I will get used to it and I could alias it if I want it to be shorten in one spec. This api also allows easy expansion to check headers and body potentially also be_an_http_gem_request and check headers and body.

@stoivo
Copy link
Author

stoivo commented Aug 2, 2023

it is released. I don't know how I can make it so you can release it too

@stoivo
Copy link
Author

stoivo commented Aug 2, 2023

I added ixti and tarcieri so some of you have release access. Somebody else I should add?

@tarcieri
Copy link
Member

tarcieri commented Aug 2, 2023

That's fine

@stoivo stoivo closed this as completed Aug 3, 2023
@ixti
Copy link
Member

ixti commented Aug 6, 2023

@stoivo can you send me an invite once again? I was off-the grid.

@stoivo
Copy link
Author

stoivo commented Aug 9, 2023

Was getting some weird errors so I removed you and added you again.

$ gem owner http-rspec --add ixti
You have enabled multi-factor authentication. Please enter OTP code.
Code:   ****
Adding ixti: User has already been taken

$ gem owner http-rspec --remove ixti
You have enabled multi-factor authentication. Please enter OTP code.
Code:   ****
The existing key doesn't have access of remove_owner on RubyGems.org. Please sign in to update access.
   Email:   simon.toivo@telhaug.no
Password:
Added remove_owner scope to the existing API key
Owner removed successfully.
Owners for gem: http-rspec
- tarcieri
- stoivo

$ gem owner http-rspec --add ixti
You have enabled multi-factor authentication. Please enter OTP code.
Code:   ***
ixti was added as an unconfirmed owner. Ownership access will be enabled after the user clicks on the confirmation mail sent to their email.
Owners for gem: http-rspec
- tarcieri
- stoivo

@ixti, did it work? if not I will send an imail to rubygems

@ixti
Copy link
Member

ixti commented Aug 28, 2023

@ixti, did it work? if not I will send an imail to rubygems

Yup, thank you!

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

No branches or pull requests

3 participants