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.

Sign up for GitHub

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

Jump to bottom

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

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
@nirvdrum
Commented out flaky spec that also wasn't testing what it was suppose…
f9fbfa7 
…d to.

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

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

This was referenced Dec 10, 2014
Merged
Merged
@nirvdrum
Copy link
Contributor Author

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

@headius
Copy link
Member

headius commented Dec 10, 2014

@nirvdrum MOAR TESTING!

@tduehr
Copy link
Contributor

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

@tduehr Can this be closed?

@tduehr
Copy link
Contributor

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
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@headius @nirvdrum @tduehr