spec: fix selinimum capture autoload issue, fixes SD-2103

fix a newly exposed autoloading edge case, that depends on spec run order.
given:

ConversationParticipant which autoloads:
  SimpleTags and
  Conversation (which would normally autoload SimpleTags),

when we reset autoloads and then resolve Conversation, ensure it restore
SimpleTags

also remove a weird self-require_dependency ... it's not a problem for AS,
but it is for selinimum

[ci selinimum capture]

test plan:
n/a, specs

Change-Id: I6b0193c9162a70554c997f4ed0c76690c86bf0e0
Reviewed-on: https://gerrit.instructure.com/102628
Tested-by: Jenkins
Reviewed-by: Landon Wilkins <lwilkins@instructure.com>
Product-Review: Landon Wilkins <lwilkins@instructure.com>
QA-Review: Landon Wilkins <lwilkins@instructure.com>
This commit is contained in:
Jon Jensen 2017-02-17 15:01:26 -07:00
parent 2e5427203f
commit 9ac6aaaee4
3 changed files with 45 additions and 31 deletions

View File

@ -77,4 +77,4 @@ class Quizzes::QuizQuestion::UserAnswer < Struct.new(:question_id, :points_possi
end
end
Dir[Rails.root + "app/models/quizzes/quiz_question/*_answer.rb"].each { |f| require_dependency f }
(Dir[Rails.root + "app/models/quizzes/quiz_question/*_answer.rb"] - [__FILE__]).each { |f| require_dependency f }

View File

@ -90,31 +90,23 @@ module Selinimum
# whenever we first autoload something, keep track of what it
# autoloads. returns an array of file paths
def track_dependencies(const_names)
nested = !dependencies_stack.empty?
dependencies_stack << []
if nested
# top-most track_dependencies call has us covered wrt resetting
yield
else
# this is a top-level autoload (e.g. the top-most `Foo` in the
# stack). to really know what this thing depends on, we need to
# reset autoloaded stuff yet again, so we don't get freeloaders
# from a previous autoload.
#
# note that we exempt nesting names, since we want to make sure
# we don't create dummy duplicate modules/classes (either due to
# redundant definitions, or rails' automatic module-from-
# directories), e.g.:
#
# # foo/bar/baz.rb
# class Foo::Bar # <- we want Baz to be the previously
# class Baz # autoloaded class, not a new one
# ...
nesting_names = const_names.map { |c| nesting_names_for(c) }.flatten
temporarily_reset_autoloads(nesting_names) { yield }
end
# when we auto-load something, to really know what it depends on,
# we need to reset autoloaded stuff, so we don't get freeloaders
# from a previous autoload. this is true both for top-level
# autoloads as well as for ones triggered by them.
#
# note that we exempt nesting names, since we want to make sure
# we don't create dummy duplicate modules/classes (either due to
# redundant definitions, or rails' automatic module-from-
# directories), e.g.:
#
# # foo/bar/baz.rb
# class Foo::Bar # <- we want Baz to be the previously
# class Baz # autoloaded class, not a new one
# ...
nesting_names = const_names.map { |c| nesting_names_for(c) }.flatten
temporarily_reset_autoloads(nesting_names) { yield }
current_dependencies
ensure
dependencies_stack.pop

View File

@ -27,7 +27,7 @@ describe Selinimum::Capture::AutoloadExtensions do
end
context "dependency tracking" do
it "temporarily resets other top-level autoloads" do
it "temporarily resets top-level sibling autoloads" do
cache_constants("SelinimumFoo") do
SelinimumFoo = Class.new
end
@ -41,17 +41,40 @@ describe Selinimum::Capture::AutoloadExtensions do
expect(Object.const_defined?("SelinimumBar")).to be true
end
it "doesn't reset nested autoloads" do
cache_constants("SelinimumFoo") do
SelinimumFoo = Class.new
it "temporarily resets nested sibling autoloads" do
cache_constants("SelinimumTopLevel") do
SelinimumTopLevel = Class.new
cache_constants("SelinimumFoo") do
SelinimumFoo = Class.new
end
cache_constants("SelinimumBar") do
expect(Object.const_defined?("SelinimumFoo")).to be true
expect(Object.const_defined?("SelinimumFoo")).to be false
SelinimumBar = Class.new
end
expect(Object.const_defined?("SelinimumFoo")).to be true
expect(Object.const_defined?("SelinimumBar")).to be true
end
end
# this will require some refactoring...
it "temporarily resets outer autoloads" do
pending "not implemented"
cache_constants("SelinimumFoo") do
SelinimumTopLevel = Class.new
cache_constants("SelinimumBar") do
expect(Object.const_defined?("SelinimumFoo")).to be false
SelinimumFoo = Class.new
end
end
expect(Object.const_defined?("SelinimumFoo")).to be true
expect(Object.const_defined?("SelinimumBar")).to be true
end
it "doesn't reset autoloads from surrounding modules" do
cache_constants("SelinimumFoo") do
SelinimumFoo = Class.new
@ -71,7 +94,6 @@ describe Selinimum::Capture::AutoloadExtensions do
SelinimumFoo = Class.new
cache_constants("SelinimumBar") do
expect(Object.const_defined?("SelinimumFoo")).to be true
SelinimumBar = Class.new
end
end