Skip to content

Commit

Permalink
Fix inheritance bug that doesn't reset state correctly
Browse files Browse the repository at this point in the history
What?
=====

This addresses a bug with a very specific set of criteria for
reproduction:

* a page object (A) inheriting from PageEz::Page with a macro declaring
  a block (nothing needs to be declared in the block)
* a page object (B) inheriting from PageEz::Page that declares a top-level
  macro named (X)
* a page object (C) inheriting from (A) that also declares a top-level
  macro named (X)

The bug that triggered this failure (a duplicate element declaration
error, which would be on (X) in the example above) was related to a
state change (setting nested_macro to true) when a block is provided to
a macro that wasn't reset after the block was complete.

When a class inherits from PageEz or its subclasses, if it's not nested,
it resets all its visitors. This key behavior wasn't being triggered on
(C), resulting in it finding the same macro (X) from (B) and determining
that it was a collision.
  • Loading branch information
joshuaclayton committed Aug 29, 2023
1 parent 733ff01 commit 07c9de7
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 0 deletions.
1 change: 1 addition & 0 deletions lib/page_ez/page.rb
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ def self.constructor_from_block(superclass = nil, &block)
klass.macro_registrar = {}
klass.class_eval(&block)
visitor.end_block_evaluation
self.nested_macro = false
end
elsif superclass
superclass
Expand Down
17 changes: 17 additions & 0 deletions spec/features/smoke_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -163,4 +163,21 @@ class PageWithComposed < BasePage
# rubocop:enable Lint/ConstantDefinitionInBlock
end.not_to raise_error
end

it "allows for inheritance where macros do not collide" do
expect do
composed_page = Class.new(PageEz::Page) do
has_one :nested, "section" do
end
end

Class.new(PageEz::Page) do
has_one :list, "ul"
end

Class.new(composed_page) do
has_one :list, "ul"
end
end.not_to raise_error
end
end

0 comments on commit 07c9de7

Please sign in to comment.