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

Use stackprof to test the lack of object allocations #896

Merged
merged 1 commit into from
May 12, 2017

Conversation

dylanahsmith
Copy link
Contributor

Problem

I was getting a bunch of warnings when running the tests that were caused by the Spy gem

/Users/dylansmith/.gem/ruby/2.3.3/gems/spy-0.4.1/lib/spy/subroutine.rb:55: warning: method redefined; discarding old any?
/Users/dylansmith/.gem/ruby/2.3.3/gems/spy-0.4.1/lib/spy/subroutine.rb:55: warning: method redefined; discarding old empty?
/Users/dylansmith/.gem/ruby/2.3.3/gems/spy-0.4.1/lib/spy/subroutine.rb:204: warning: instance variable @plan not initialized
/Users/dylansmith/.gem/ruby/2.3.3/gems/spy-0.4.1/lib/spy/subroutine.rb:72: warning: method redefined; discarding old any?
/Users/dylansmith/.gem/ruby/2.3.3/gems/spy-0.4.1/lib/spy/subroutine.rb:228: warning: previous definition of any? was here
/Users/dylansmith/.gem/ruby/2.3.3/gems/spy-0.4.1/lib/spy/subroutine.rb:72: warning: method redefined; discarding old empty?
/Users/dylansmith/.gem/ruby/2.3.3/gems/spy-0.4.1/lib/spy/subroutine.rb:228: warning: previous definition of empty? was here

Solution

We were just using the spy gem in a single test, where we were testing the implementation instead of what we actually cared about, avoiding object allocations. I changed the test to use stackprof to count object allocations so we could assert that none were made.

This also fixes the Gemfile so that we can use stackprof to profile any any of the support MRI versions. Previously it would only install stackprof for MRI ruby 2.1, which is now the minimum ruby version we support.

Verified

This commit was signed with the committer’s verified signature.
scorpionknifes Cheng-Zhen (CZ) Yang
@dylanahsmith dylanahsmith requested a review from jasonhl May 11, 2017 19:29
Copy link
Collaborator

@jasonhl jasonhl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much better, thank you.

@dylanahsmith dylanahsmith merged commit 48a6d86 into master May 12, 2017
@dylanahsmith dylanahsmith deleted the remove-spy branch May 12, 2017 13:20
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.

None yet

2 participants