Dduvall has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/171985

Change subject: Instance evaluate blocks for new environments
......................................................................

Instance evaluate blocks for new environments

Using instance evaluation for new environment contexts gives us a more
compact DSL that's more congruent with how Cucumber evaluates its own
step definitions. It also allows the user to receive the overridden
configuration values as block arguments.

It changes the following usage:

  When(/.../) do
    on_wiki(:b) do |env|
      as_user(:a) do |env|
        env.visit(LoginPage).login_with(env[:mediawiki_user], ...)
      end
    end
  end

To a more natural and succinct:

  When(/.../) do
    on_wiki(:b) do
      as_user(:a) do |user, pass|
        visit(LoginPage).login_with(user, pass)
      end
    end
  end

Change-Id: Id691e3a77c72445cfa9d3f4a2cbc270589703c6e
---
M lib/mediawiki_selenium/environment.rb
M spec/environment_spec.rb
2 files changed, 111 insertions(+), 53 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/selenium 
refs/changes/85/171985/1

diff --git a/lib/mediawiki_selenium/environment.rb 
b/lib/mediawiki_selenium/environment.rb
index 737bf2d..eb6848e 100644
--- a/lib/mediawiki_selenium/environment.rb
+++ b/lib/mediawiki_selenium/environment.rb
@@ -55,8 +55,9 @@
     #
     # @param id [Symbol] Alternative user ID.
     #
-    # @yield [env]
-    # @yieldparam env [Environment] Environment
+    # @yield [user, password]
+    # @yieldparam user [String] Alternative MediaWiki user.
+    # @yieldparam password [String] Alternative MediaWiki password.
     #
     # @return [Environment]
     #
@@ -138,8 +139,7 @@
     # @param id [Symbol] Browser session ID.
     # @param overrides [Hash] Browser configuration overrides.
     #
-    # @yield [env]
-    # @yieldparam env [Environment] Environment
+    # @yield [*args] Overridden browser configuration.
     #
     # @return [Environment]
     #
@@ -201,10 +201,14 @@
     # Executes the given block within the context of an environment that's
     # using the given alternative wiki URL and its corresponding API endpoint.
     #
+    # @example Visit a random page on wiki B
+    #   on_wiki(:b) { visit(RandomPage) }
+    #
     # @param id [Symbol] Alternative wiki ID.
     #
-    # @yield [env]
-    # @yieldparam env [Environment] Environment
+    # @yield [wiki_url, api_url]
+    # @yieldparam wiki_url [String] Alternative wiki URL.
+    # @yieldparam api_url [String] Alternative API URL.
     #
     # @return [Environment]
     #
@@ -258,15 +262,15 @@
     #
     # @param id [Symbol] Alternative wiki ID.
     #
-    # @yield [env]
-    # @yieldparam env [Environment] Environment
+    # @yield [url]
+    # @yieldparam url [String] Wiki URL.
     #
     # @return [Environment]
     #
     def visit_wiki(id, &blk)
-      on_wiki(id) do |env|
-        browser.goto env.wiki_url
-        blk.call(env) unless blk.nil?
+      on_wiki(id) do |url|
+        browser.goto url
+        instance_exec(url, &blk) unless blk.nil?
       end
     end
 
@@ -298,28 +302,30 @@
       url
     end
 
-    # Yields a new environment using the alternative versions of the given
-    # configuration options. The alternative values are resolved by looking up
-    # options that correspond to the given ones but have the given ID
-    # appended.
+    # Executes the given block within the context of a new environment
+    # configured using the alternative versions of the given options. The
+    # alternative configuration values are resolved using the given ID and
+    # passed to the block as arguments.
     #
     # @example Overwrite :foo with the :b alternative
-    #   # given an environment with { foo: "x", foo_b: "y", ... }
-    #   env.with_alternative(:foo, :b) do |env|
-    #     env # => { foo: "y", ... }
+    #   # given an environment with config { foo: "x", foo_b: "y", ... }
+    #   with_alternative(:foo, :b) do |foo|
+    #     self # => #<Environment @config = { foo: "y", ... }>
+    #     foo # => "y"
     #   end
     #
     # @example Overwrite both :foo and :bar with the :b alternatives
-    #   # given { foo: "x", foo_b: "y", bar: "w", bar_b: "z" }
-    #   env.with_alternative([:foo, :bar], :b) do |env|
-    #     env # => { foo: "y", bar: "z", ... }
+    #   # given an environment with config { foo: "x", foo_b: "y", bar: "w", 
bar_b: "z" }
+    #   with_alternative([:foo, :bar], :b) do |foo, bar|
+    #     self # => #<Environment @config = { foo: "y", bar: "z", ... }>
+    #     foo # => "y"
+    #     bar # => "z"
     #   end
     #
-    # @param names [Array|Symbol] Configuration option or options.
+    # @param names [Symbol|Array<Symbol>] Configuration option or options.
     # @param id [Symbol] Alternative user ID.
     #
-    # @yield [env]
-    # @yieldparam env [Environment] The modified environment.
+    # @yield [*args] Values of the overridden configuration.
     #
     # @return [Environment] The modified environment.
     #
@@ -337,10 +343,12 @@
       hash.each.with_object({}) { |(k, v), acc| acc[k.to_s.downcase.to_sym] = 
v }
     end
 
-    def with(overrides = {})
+    def with(overrides = {}, &blk)
+      overrides = normalize_config(overrides)
+
       clone.tap do |env|
-        env.config.merge!(normalize_config(overrides))
-        yield env
+        env.config.merge!(overrides)
+        env.instance_exec(*overrides.values, &blk) unless blk.nil?
       end
     end
   end
diff --git a/spec/environment_spec.rb b/spec/environment_spec.rb
index 6b372f1..253c6d3 100644
--- a/spec/environment_spec.rb
+++ b/spec/environment_spec.rb
@@ -44,6 +44,8 @@
     end
 
     describe "#as_user" do
+      subject { env.as_user(:b) {} }
+
       let(:config) do
         {
           mediawiki_user: "user",
@@ -53,12 +55,21 @@
         }
       end
 
-      it "yields a new environment for the alternative user and its password" 
do
-        expected_env = Environment.new(config.merge(
+      let(:new_env) { double(Environment) }
+      let(:new_config) { double(Hash) }
+
+      before do
+        expect(env).to receive(:clone).and_return(new_env)
+        expect(new_env).to receive(:config).and_return(new_config)
+      end
+
+      it "executes in the new environment for the alternative user and its 
password" do
+        expect(new_config).to receive(:merge!).with(
           mediawiki_user: "user b",
           mediawiki_password: "pass b",
-        ))
-        expect { |block| env.as_user(:b, &block) }.to 
yield_with_args(expected_env)
+        )
+        expect(new_env).to receive(:instance_exec).with("user b", "pass b")
+        subject
       end
     end
 
@@ -161,27 +172,39 @@
     end
 
     describe "#in_browser" do
-      subject { env.in_browser(id, overrides, &blk) }
+      subject { env.in_browser(id, overrides) {} }
 
       let(:id) { :a }
       let(:overrides) { {} }
 
-      it "yields an environment with a new browser session" do
-        expected_env = Environment.new(config.merge(_browser_session: id))
-        expect { |block| env.in_browser(id, overrides, &block) }.to 
yield_with_args(expected_env)
+      let(:new_env) { double(Environment) }
+      let(:new_config) { double(Hash) }
+
+      before do
+        expect(env).to receive(:clone).and_return(new_env)
+        expect(new_env).to receive(:config).and_return(new_config)
+      end
+
+      it "executes in the new environment with a new browser session" do
+        expect(new_config).to receive(:merge!).with(_browser_session: id)
+        expect(new_env).to receive(:instance_exec).with(id)
+        subject
       end
 
       context "given browser configuration overrides" do
         let(:overrides) { { language: "eo" } }
 
-        it "yields an environment with the prefixed overrides" do
-          expected_env = Environment.new(config.merge(_browser_session: id, 
browser_language: "eo"))
-          expect { |block| env.in_browser(id, overrides, &block) }.to 
yield_with_args(expected_env)
+        it "executes in the new environment with the prefixed overrides" do
+          expect(new_config).to receive(:merge!).with(_browser_session: id, 
browser_language: "eo")
+          expect(new_env).to receive(:instance_exec).with("eo", id)
+          subject
         end
       end
     end
 
     describe "#on_wiki" do
+      subject { env.on_wiki(:b) {} }
+
       let(:config) do
         {
           mediawiki_url: "http://an.example/wiki";,
@@ -191,12 +214,24 @@
         }
       end
 
-      it "yields a new environment for the alternative wiki and API urls" do
-        expected_env = Environment.new(config.merge(
+      let(:new_env) { double(Environment) }
+      let(:new_config) { double(Hash) }
+
+      before do
+        expect(env).to receive(:clone).and_return(new_env)
+        expect(new_env).to receive(:config).and_return(new_config)
+      end
+
+      it "executes in the new environment using the alternative wiki and API 
urls" do
+        expect(new_config).to receive(:merge!).with(
           mediawiki_url: "http://alt.example/wiki";,
-          mediawiki_api_url: "http://alt.example/api";,
-        ))
-        expect { |block| env.on_wiki(:b, &block) }.to 
yield_with_args(expected_env)
+          mediawiki_api_url: "http://alt.example/api";
+        )
+        expect(new_env).to receive(:instance_exec).with(
+          "http://alt.example/wiki";,
+          "http://alt.example/api";
+        )
+        subject
       end
     end
 
@@ -248,6 +283,8 @@
     end
 
     describe "#with_alternative" do
+      subject { env.with_alternative(names, id) {} }
+
       let(:config) do
         {
           mediawiki_url: "http://an.example/wiki";,
@@ -257,26 +294,39 @@
         }
       end
 
+      let(:new_env) { double(Environment) }
+      let(:new_config) { double(Hash) }
+
+      before do
+        expect(env).to receive(:clone).and_return(new_env)
+        expect(new_env).to receive(:config).and_return(new_config)
+      end
+
       context "given one option name and an ID" do
         let(:names) { :mediawiki_url }
+        let(:id) { :b }
 
-        it "yields an environment that substitutes it using the alternative" do
-          expected_env = Environment.new(config.merge(
-            mediawiki_url: "http://alt.example/wiki";,
-          ))
-          expect { |block| env.with_alternative(names, :b, &block) }.to 
yield_with_args(expected_env)
+        it "executes in the new environment that substitutes it using the 
alternative" do
+          expect(new_config).to receive(:merge!).with(mediawiki_url: 
"http://alt.example/wiki";)
+          expect(new_env).to 
receive(:instance_exec).with("http://alt.example/wiki";)
+          subject
         end
       end
 
       context "given multiple option names and an ID" do
         let(:names) { [:mediawiki_url, :mediawiki_api_url] }
+        let(:id) { :b }
 
-        it "yields an environment that substitutes both using the 
alternatives" do
-          expected_env = Environment.new(config.merge(
+        it "executes in the new environment that substitutes both using the 
alternatives" do
+          expect(new_config).to receive(:merge!).with(
             mediawiki_url: "http://alt.example/wiki";,
-            mediawiki_api_url: "http://alt.example/api";,
-          ))
-          expect { |block| env.with_alternative(names, :b, &block) }.to 
yield_with_args(expected_env)
+            mediawiki_api_url: "http://alt.example/api";
+          )
+          expect(new_env).to receive(:instance_exec).with(
+            "http://alt.example/wiki";,
+            "http://alt.example/api";
+          )
+          subject
         end
       end
     end

-- 
To view, visit https://gerrit.wikimedia.org/r/171985
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Id691e3a77c72445cfa9d3f4a2cbc270589703c6e
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/selenium
Gerrit-Branch: env-abstraction-layer
Gerrit-Owner: Dduvall <[email protected]>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to