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

Add ruby3 support #38

Closed
wants to merge 4 commits into from
Closed

Conversation

kvokka
Copy link

@kvokka kvokka commented Jun 5, 2021

Removed EOL Ruby versions and added actual instead
Keep in mind, that the most recent versions require TLS in SMTP requests, which required to update a couple of requests.
Made specs green again

Added Ruby 3 Ractor support (technically lib/tcr/recordable_tcp_socket.rb#13 was the only thing which i initially needed)

@mgruner
Copy link

mgruner commented Mar 23, 2022

@kvokka we have issues with this part where tcr.rb overrides Socket.tcp:

class Socket
  class << self
    alias_method :real_tcp, :tcp

    def tcp(host, port, *socket_opts)
      if TCR.configuration.hook_tcp_ports.include?(port)
        TCR::RecordableTCPSocket.new(host, port, TCR.cassette)
      else
        real_tcp(host, port, *socket_opts)
      end
    end
  end
end

In Ruby 3, that needs to be **socket_opts rather than *socket_opts. It yields TypeError: no implicit conversion of Hash into String. Occurs when calling Net::IMAP.new.

Btw, it seems that the owner of this repository @robforman is not active on GitHub anymore since 2018. Maybe tcr needs to be forked?

@robforman
Copy link
Contributor

@mgruner I'm not as active. Want to be a collaborator on the project?

@mgruner
Copy link

mgruner commented Mar 25, 2022

@robforman thank you for your proposal, I needed a moment to reflect on it.

Can I support you in getting tcr ready for Ruby 3, handling valid incoming PRs, and keeping it alive for future versions, including gem releases? Yes.

If that's fine for you, feel free to add me here and possibly on rubygems.org (username mrtngrnr). Thanks!

@w-A-L-L-e
Copy link

**socket_opts still gives errors for me when using ldap bind.
This works (and as I only use this gem in development, I dont mind the connect timeout and resolv timeout to be stripped):

class Socket
  class << self
    alias_method :real_tcp, :tcp

    def tcp(host, port, *socket_opts)
      if TCR.configuration.hook_tcp_ports.include?(port)
        TCR::RecordableTCPSocket.new(host, port, TCR.cassette)
      else
        # PATCHED for ruby 3.1.2, **socket_opts gives less errors, but still breaks on bind
        # for now, just omit the extra 2 options entirely seems to fix most issues for now:
        # real_tcp(host, port, *socket_opts)
        real_tcp(host, port)
      end
    end
  end
end

@sionide21
Copy link
Member

Fixed in #41

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.

5 participants