Audit and convert tests to test/unit (minitest) #121
Conversation
| # on them to work. Pipes are more robust for this test, so we'll skip | ||
| # the #connect call since it fails. | ||
| flexmock(OpenSSL::SSL::SSLSocket). | ||
| new_instances.should_receive(:connect => nil) |
There was a problem hiding this comment.
I'd like to remove flexmock and only use minitest/mock here, but I had some difficulty converting this test. It's not high priority, but if someone is interested in tackling it, feel free to talk to me.
There was a problem hiding this comment.
I'd like to rebuild this test to use a real socket instead of a pipe since this is why JRuby CI builds fail (mismatch between pipe and socket; thanks strong Java types/Obama). If this is passing as is, let's worry about replacing the mock and pipe later, and I'll assign myself to that.
| search_result_ber.ber_identifier = Net::LDAP::PDU::SearchResult | ||
| search_result = [2, search_result_ber] | ||
| @tcp_socket.should_receive(:read_ber).and_return(search_data). | ||
| and_return(search_result) |
There was a problem hiding this comment.
Would love to simplify this kind of construct or provide helpers at least. This general pattern is all over the tests.
Otherwise, we conflict with Net::LDAP instrumentation test class, causing mysterious failures.
|
|
||
| def raw_string(s) | ||
| # Conveniently, String#b only needs to be called when it exists | ||
| s.respond_to?(:b) ? s.b : s |
There was a problem hiding this comment.
@jch when moving the BER tests, we switched to using String#b but that is 2.0+ only and breaks the 1.9 build. We can either monkeypatch .b support (blech), bring this back, or work something a bit better out.
There was a problem hiding this comment.
Fine with bringing this back. Took it out because I didn't know what it was for. I'll document that it's for 1.9 also.
No strong reason, it was already in use, so I left it there. I've added |
Conflicts: net-ldap.gemspec
|
Unfortunately, flexmock doesn't have minitest support out of the box. There are 14 references to flexmock in the test suite. Ideally we'd convert these to use |
This reverts commit 1168215. ruby-ldap#121 (comment)
|
@jch bah, OK. No worries, thanks for giving it a go. Don't mind taking point on that in a followup. |
|
@jch alright, finished up the last bits. Give it look over and merge it when you're ready. |
|
Still some failures with jruby-19mode, but we allow failures for that part of the build matrix. 🚢 ing! |
Audit and convert tests to test/unit (minitest)
|
@jch those failures are consistent with master before this branch was On Tuesday, October 14, 2014, Jerry Cheung notifications@github.com wrote:
Matt Todd |
This reverts commit 1168215. ruby-ldap#121 (comment)
Audit and convert tests to test/unit (minitest)
🚧 WIP 🚧
This PR converts the RSpec tests to minitest. Most of the tests were translated over, but I'll make inline notes where I modified or removed tests for clarity.