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

Improve FFI::Function can wrap a blocking function spec #2299

Open
nirvdrum opened this issue Dec 10, 2014 · 6 comments
Open

Improve FFI::Function can wrap a blocking function spec #2299

nirvdrum opened this issue Dec 10, 2014 · 6 comments

Comments

@nirvdrum
Copy link
Contributor

@nirvdrum nirvdrum commented Dec 10, 2014

The spec for FFI::Function wrapping a blocking function has proven to be problematic on Travis, where tests can take longer than the 1 second threshold. One "fix" is to increase the threshold. In speaking with @tduehr, this whole spec seems bad though and thus should be revisited.

For posterity, here is the failure from https://travis-ci.org/jruby/jruby/jobs/43403371:

[exec] 1) FFI::Function can wrap a blocking function
[exec] Failure/Error: threads << Thread.new(time) { expect(Time.now - time).to be < 1 }
[exec] expected: < 1
[exec] got: 1.49
[exec] # ./spec/ffi/function_spec.rb:70:in `(root)'
nirvdrum added a commit that referenced this issue Dec 10, 2014
…d to.

See #2299: 'Improve FFI::Function can wrap a blocking function spec' for details.
@tduehr
Copy link
Contributor

@tduehr tduehr commented Dec 10, 2014

how's this look?

  it 'can wrap a blocking function' do
    fp = FFI::Function.new(:void, [ :int ], @libtest.find_function('testBlocking'), :blocking => true)
    threads = 10.times.map do |x|
      Thread.new do
        time = Time.now
        fp.call(2)
        expect(Time.now - time).to be > 2
      end
    end
    threads.each { |t| t.join }
  end

@nirvdrum
Copy link
Contributor Author

@nirvdrum nirvdrum commented Dec 10, 2014

It looks fine to me. But what's the significance of running it 10 times?

@headius
Copy link
Member

@headius headius commented Dec 10, 2014

@nirvdrum MOAR TESTING!

@tduehr
Copy link
Contributor

@tduehr tduehr commented Dec 11, 2014

just trying to make sure it gets caught even if under heavy-ish load... still not the greatest but...

@nirvdrum
Copy link
Contributor Author

@nirvdrum nirvdrum commented Jan 15, 2015

@tduehr Can this be closed?

@tduehr
Copy link
Contributor

@tduehr tduehr commented Jan 16, 2015

I really don't know... This version should be better but I don't know if it's actually correct or there isn't a better one...

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