Skip to content

Commit

Permalink
Merge pull request #1178 from rspec/remove-cache-busters-part-2
Browse files Browse the repository at this point in the history
Remove cache busters part 2
  • Loading branch information
myronmarston committed Nov 14, 2013
2 parents 227d668 + a1346c2 commit d2ef379
Show file tree
Hide file tree
Showing 9 changed files with 97 additions and 33 deletions.
2 changes: 1 addition & 1 deletion lib/rspec/core/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -861,7 +861,7 @@ def setup_load_path_and_require(paths)
# @private
if RUBY_VERSION.to_f >= 1.9
def safe_extend(mod, host)
host.extend(mod) unless (class << host; self; end) < mod
host.extend(mod) unless host.singleton_class < mod
end
else
def safe_extend(mod, host)
Expand Down
9 changes: 6 additions & 3 deletions lib/rspec/core/filter_manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -110,14 +110,14 @@ def without_conditional_filters
module BackwardCompatibility
def merge(orig, opposite, *updates)
_warn_deprecated_keys(updates.last)
super
end

def reverse_merge(orig, opposite, *updates)
_warn_deprecated_keys(updates.last)
super
end

private

# Supports a use case that probably doesn't exist: overriding the
# if/unless procs.
def _warn_deprecated_keys(updates)
Expand All @@ -133,12 +133,13 @@ def _warn_deprecated_key(key, updates)
end
end

include BackwardCompatibility

attr_reader :exclusions, :inclusions

def initialize
@exclusions = ExclusionFilterHash.new
@inclusions = InclusionFilterHash.new
extend(BackwardCompatibility)
end

def add_location(file_path, line_numbers)
Expand Down Expand Up @@ -198,6 +199,7 @@ def unless_standalone(*args)
end

def merge(orig, opposite, *updates)
super
orig.merge!(updates.last).each_key {|k| opposite.delete(k)}
end

Expand All @@ -207,6 +209,7 @@ def replace(orig, opposite, *updates)
end

def reverse_merge(orig, opposite, *updates)
super
updated = updates.last.merge(orig)
opposite.each_pair {|k,v| updated.delete(k) if updated[k] == v}
orig.replace(updated)
Expand Down
4 changes: 2 additions & 2 deletions lib/rspec/core/shared_example_group.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,10 @@ def add_group(source, *args, &block)

unless args.empty?
mod = Module.new
(class << mod; self; end).send :define_method, :extended do |host|
(class << mod; self; end).send :define_method, :included do |host|
host.class_eval(&block)
end
RSpec.configuration.extend mod, *args
RSpec.configuration.include mod, *args
end
end

Expand Down
42 changes: 42 additions & 0 deletions script/ignores
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,45 @@ lib/rspec/core/shared_example_group.rb: alias_method :shared_examples_for
# The `alias_method` calls below happen when users define a named let.
# It happens at spec definition time, not spec run time.
lib/rspec/core/memoized_helpers.rb: alias_method :subject, name

# These `const_set` calls happen at spec definition time,
# not at spec run time, so they are OK.
lib/rspec/core/example_group.rb: const_scope.const_set(name, group)
lib/rspec/core/memoized_helpers.rb: example_group.const_set(:NamedSubjectPreventSuper, self)
lib/rspec/core/memoized_helpers.rb: example_group.const_set(:LetDefinitions, mod)

# These mentions of `extend` get executed at spec definition time
# (unless the user changes their RSpec config at spec run time, but
# there's no way to work around that).
lib/rspec/core/configuration.rb: def extend(mod, \*filters)
lib/rspec/core/configuration.rb: include_or_extend_modules << \[:extend, mod, Metadata.build_hash_from(filters)\]
lib/rspec/core/configuration.rb: host.extend(mod) unless host.singleton_class < mod
lib/rspec/core/configuration.rb: host.extend(mod) unless (class << host; self; end).included_modules.include?(mod)
lib/rspec/core/configuration.rb: extend RSpec::SharedContext
lib/rspec/core/dsl.rb:extend RSpec::Core::DSL
lib/rspec/core/example_group.rb: extend Hooks
lib/rspec/core/example_group.rb: extend SharedExampleGroup
lib/rspec/core/memoized_helpers.rb: mod.extend(ClassMethods)
lib/rspec/core/shared_example_group.rb:extend RSpec::Core::SharedExampleGroup::TopLevelDSL

# These uses of `extend` would be nice to get rid of, but our prior attempts
# to convert the metadata hash to a composition-based design have created perf
# regressions. These lines also only are only executed when an example or
# example group is defined, so they shouldn't ever blow the method cache
# at spec run time.
lib/rspec/core/metadata.rb: store(:example_group, {:example_group => parent_group_metadata\[:example_group\].extend(GroupMetadataHash)}.extend(GroupMetadataHash))
lib/rspec/core/metadata.rb: store(:example_group, {}.extend(GroupMetadataHash))
lib/rspec/core/metadata.rb: dup.extend(ExampleMetadataHash).configure_for_example(description, user_metadata)

# This use of `extend` only happens on 1.8.7, when a shared example group
# is defined (to provide the Proc#source_location method on the provided
# block). It should only happen at spec definition time (unless users are
# defining shared example groups at spec run time).
lib/rspec/core/shared_example_group.rb: block.extend Module.new {

# This happens when an example group is defined, not at spec run time.
lib/rspec/core/example_group.rb: subclass = Class.new(parent)

# This happens at file load time.
lib/rspec/core/formatters/deprecation_formatter.rb: DeprecationError = Class.new(StandardError)

23 changes: 23 additions & 0 deletions script/list_method_cache_busters.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
#!/bin/bash
# set -x

# This list is from https://charlie.bz/blog/things-that-clear-rubys-method-cache

IGNORE_FILE=/tmp/cache_busters_ignore
COMMENT_LINE_RE="^(\w|\/)+\.rb: +#"

cat script/ignores | grep -v "^$" | ruby -ne 'puts $_.split(/\s+###/)[0]' > $IGNORE_FILE

egrep 'def [a-z]*\..*' -R lib | grep -v "def self" | grep -v -f $IGNORE_FILE | egrep -v "$COMMENT_LINE_RE"
grep undef -R lib | grep -v -f $IGNORE_FILE | egrep -v "$COMMENT_LINE_RE"
grep alias_method -R lib | grep -v -f $IGNORE_FILE | egrep -v "$COMMENT_LINE_RE"
grep remove_method -R lib | grep -v -f $IGNORE_FILE | egrep -v "$COMMENT_LINE_RE"
grep const_set -R lib | grep -v -f $IGNORE_FILE | egrep -v "$COMMENT_LINE_RE"
grep remove_const -R lib | grep -v -f $IGNORE_FILE | egrep -v "$COMMENT_LINE_RE"
egrep '\bextend\b' -R lib | grep -v -f $IGNORE_FILE | egrep -v "$COMMENT_LINE_RE"
grep 'Class.new' -R lib | grep -v -f $IGNORE_FILE | egrep -v "$COMMENT_LINE_RE"
grep private_constant -R lib | grep -v -f $IGNORE_FILE | egrep -v "$COMMENT_LINE_RE"
grep public_constant -R lib | grep -v -f $IGNORE_FILE | egrep -v "$COMMENT_LINE_RE"
grep "Marshal.load" -R lib | grep -v -f $IGNORE_FILE | egrep -v "$COMMENT_LINE_RE"
grep "OpenStruct.new" -R lib | grep -v -f $IGNORE_FILE | egrep -v "$COMMENT_LINE_RE"

21 changes: 0 additions & 21 deletions script/method_cache_busters.sh

This file was deleted.

15 changes: 15 additions & 0 deletions script/prevent_runtime_method_cache_busters
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#!/usr/bin/env ruby

method_cache_busters = `script/list_method_cache_busters.sh`.split("\n").map(&:chomp)

if method_cache_busters.any?
puts "=" * 80
puts "Found #{method_cache_busters.size} new constructs that bust the method cache."
puts "These should be eliminated or added to the `ignores` file."
puts
puts "For more information, see https://charlie.bz/blog/things-that-clear-rubys-method-cache"
puts
puts method_cache_busters.join("\n")
puts "=" * 80
exit(1)
end
2 changes: 2 additions & 0 deletions script/test_all
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ fi
# https://github.com/carlhuda/bundler/issues/2382
bundle exec bin/cucumber --strict

script/prevent_runtime_method_cache_busters

# Test against other RSpec gems.

# Delete the directory for idempotency when running locally
Expand Down
12 changes: 6 additions & 6 deletions spec/rspec/core/shared_example_group_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,12 @@ module SharedExampleGroup
end

context "given a hash" do
it "delegates extend on configuration" do
it "delegates include on configuration" do
implementation = Proc.new { def bar; 'bar'; end }
define_shared_group(:foo => :bar, &implementation)
a = RSpec.configuration.include_or_extend_modules.first
expect(a[0]).to eq(:extend)
expect(Class.new.extend(a[1]).new.bar).to eq('bar')
expect(a[0]).to eq(:include)
expect(Class.new.send(:include, a[1]).new.bar).to eq('bar')
expect(a[2]).to eq(:foo => :bar)
end
end
Expand All @@ -89,12 +89,12 @@ module SharedExampleGroup
expect(SharedExampleGroup.registry.shared_example_groups[group]["name"]).to eq implementation
end

it "delegates extend on configuration" do
it "delegates include on configuration" do
implementation = Proc.new { def bar; 'bar'; end }
define_shared_group("name", :foo => :bar, &implementation)
a = RSpec.configuration.include_or_extend_modules.first
expect(a[0]).to eq(:extend)
expect(Class.new.extend(a[1]).new.bar).to eq('bar')
expect(a[0]).to eq(:include)
expect(Class.new.send(:include, a[1]).new.bar).to eq('bar')
expect(a[2]).to eq(:foo => :bar)
end
end
Expand Down

0 comments on commit d2ef379

Please sign in to comment.