Project

General

Profile

Actions

Feature #7400

closed

Incorporate OpenSSL tests from JRuby.

Added by zzak (zzak _) over 11 years ago. Updated over 2 years ago.

Status:
Closed
Assignee:
-
Target version:
-
[ruby-core:49566]

Description

=begin
from github: https://github.com/ruby/ruby/pull/206

((These are tests we added to jruby-openssl over the years. They did not have equivalents in Ruby's test suite, so we are hoping to contribute them back to help build up MRI's suite as a common suite.))

((Some caveats:))

  • ((Not all of these pass in MRI's OpenSSL impl. Many differences appear to be minor, but I need help sorting out what's wrong in MRI or JRuby or minor enough that the test just needs to be patched.))
  • ((There are several tests that reference bug numbers from JRuby. We would like to contribute these tests, but we would ideally not lose the JRuby bug numbers for future reference.))
  • ((This commit also adds fixtures for the certificate tests, using some self-generated certs, keys, etc.))

((I am standing by to work with ruby-core on getting these tests incorporated.))
=end


Files

openssl_tests_from_jruby.patch (245 KB) openssl_tests_from_jruby.patch zzak (zzak _), 11/19/2012 01:10 PM

Updated by MartinBosslet (Martin Bosslet) over 11 years ago

zzak (Zachary Scott) wrote:

=begin
from github: https://github.com/ruby/ruby/pull/206

((These are tests we added to jruby-openssl over the years. They did not have equivalents in Ruby's test suite, so we are hoping to contribute them back to help build up MRI's suite as a common suite.))

Thank you, I really like and appreciate the idea.

((Some caveats:))

  • ((Not all of these pass in MRI's OpenSSL impl. Many differences appear to be minor, but I need help sorting out what's wrong in MRI or JRuby or minor enough that the test just needs to be patched.))

OK, I will run the tests on my machine and publish the report here. We could discuss our next actions based on the results then?

  • ((There are several tests that reference bug numbers from JRuby. We would like to contribute these tests, but we would ideally not lose the JRuby bug numbers for future reference.))

I'm confused, I was thinking I could simply strip those references when committing it to CRuby, or not?

  • ((This commit also adds fixtures for the certificate tests, using some self-generated certs, keys, etc.))

Hmm, it might be worth the effort trying to combine what we have in an attempt to refactor and clean up. There's already similar stuff in our test suite, my bet is there will probably be some redundancies. I'd be happy to work with you to clean this up!

((I am standing by to work with ruby-core on getting these tests incorporated.))

Again, thanks for your efforts!

Updated by mame (Yusuke Endoh) over 11 years ago

  • Status changed from Open to Assigned

Adding tests is always welcome. Thank you!

Martin, you can commit it even after preview2 release if it contains only tests.
(Of course, it is much better to include it in preview2.)
But, note that any expected failure should not be left; it will make the rubyci result difficult to see.

--
Yusuke Endoh

Updated by mame (Yusuke Endoh) over 11 years ago

  • Priority changed from Normal to 3

Let me set the priority to low to distinguish other 2.0.0 mandatory tickets.

--
Yusuke Endoh

Updated by headius (Charles Nutter) over 11 years ago

MartinBosslet (Martin Bosslet) wrote:

((Some caveats:))

  • ((Not all of these pass in MRI's OpenSSL impl. Many differences appear to be minor, but I need help sorting out what's wrong in MRI or JRuby or minor enough that the test just needs to be patched.))

OK, I will run the tests on my machine and publish the report here. We could discuss our next actions based on the results then?

Sounds good. I will monitor this bug, but poke me if I haven't noticed updates.

  • ((There are several tests that reference bug numbers from JRuby. We would like to contribute these tests, but we would ideally not lose the JRuby bug numbers for future reference.))

I'm confused, I was thinking I could simply strip those references when committing it to CRuby, or not?

I guess there's no good way to keep JRuby bug information present in those issues. Can we at least make sure the essence of the bug's description is captured in a one-line comment for those tests? It would be nice to see more comments in MRI tests indicating what they're testing in natural language.

  • ((This commit also adds fixtures for the certificate tests, using some self-generated certs, keys, etc.))

Hmm, it might be worth the effort trying to combine what we have in an attempt to refactor and clean up. There's already similar stuff in our test suite, my bet is there will probably be some redundancies. I'd be happy to work with you to clean this up!

That sounds just fine. I think we added stuff, MRI added stuff, and now merging together requires a little cleanup. I'm just not a crypto guy so I will have trouble determining which tests are redundant.

As far as cert/key fixtures go, there's very few tests that wouldn't work with any old cert or key...so that's worth cleaning up for sure.

As a subsequent step, it would be worthwhile to port some of these tests (with additional cleanup) into RubySpec.

Updated by ko1 (Koichi Sasada) about 11 years ago

  • Target version changed from 2.0.0 to 2.1.0

ping -> Martin

Updated by MartinBosslet (Martin Bosslet) about 11 years ago

I'm sorry I couldn't make this happen in time for 2.0.0. I will try to sync with the JRuby devs to unify the tests for 2.1.0.

Updated by hsbt (Hiroshi SHIBATA) about 10 years ago

  • Target version changed from 2.1.0 to 2.2.0

Updated by hsbt (Hiroshi SHIBATA) almost 10 years ago

[1/8] TestCertificate#test_cert_extensions = 4.90 s
  1) Failure:
TestCertificate#test_cert_extensions [/ruby/path/test/openssl/test_certificate.rb:72]:
<"keyid:80:14:24:D1:34:18:66:91:2A:63:76:AA:19:CE:17:20:56:56:5E:10:8F:AA"> expected but was
<"keyid:24:D1:34:18:66:91:2A:63:76:AA:19:CE:17:20:56:56:5E:10:8F:AA\nDirName:/C=JP/O=ctor.org/OU=Development/CN=http-access2\nserial:01\n">.

  2) Failure:
TestCertificate#test_cert_extensions [./test/runner.rb:24]:
Expected [[57245, #<Process::Status: pid 57245 exit 0>],
 [57250, #<Process::Status: pid 57250 exit 0>]] to be empty.

[7/8] TestCertificate#test_to_pem_with_empty_object = 0.00 s
  3) Failure:
TestCertificate#test_to_pem_with_empty_object [/ruby/path/test/openssl/test_certificate.rb:101]:
<"MAA="> expected but was
<"MBQwDQYJKoZIhvcNAQEBBQADAwAwAA==">.

I applied this patch and run it. but some test results is fail.

Charles,

Could you investigate this?

Updated by zzak (zzak _) over 9 years ago

  • Assignee changed from MartinBosslet (Martin Bosslet) to headius (Charles Nutter)
  • Priority changed from 3 to Normal
Actions #10

Updated by zzak (zzak _) over 8 years ago

  • Assignee changed from headius (Charles Nutter) to 7150

Updated by hsbt (Hiroshi SHIBATA) over 2 years ago

  • Status changed from Assigned to Closed

We continue to discuss this at https://github.com/ruby/openssl/issues/20

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0