test: separate remote tests into own directory

Dropped the webmock dependency. We have our own http adapter. So we can
stub that to inject a mock. As an added bonus this does not mess with
other http requests.

Also wrote down testing strategy. Not completely implemented yet.

12 files changed, 189 insertions, 154 deletions

diff --git a/lib/nickserver/nicknym/source.rb b/lib/nickserver/nicknym/source.rb
index 3957cdd..0776a36 100644
--- a/lib/nickserver/nicknym/source.rb
+++ b/lib/nickserver/nicknym/source.rb
@@ -19,7 +19,7 @@ module Nickserver
 
       def get(*args)
         args[0] = "https://#{args.first}"
-        adapter.get *args
+        adapter.get(*args)
       end
     end
   end
diff --git a/lib/nickserver/reel_server.rb b/lib/nickserver/reel_server.rb
index b681577..d2a95ba 100644
--- a/lib/nickserver/reel_server.rb
+++ b/lib/nickserver/reel_server.rb
@@ -32,6 +32,7 @@ module Nickserver
     protected
 
     def handler_for(request)
+      # with reel the request is the responder
       Dispatcher.new(request)
     end
 
diff --git a/nickserver.gemspec b/nickserver.gemspec
index 9338b3c..ac20a3f 100644
--- a/nickserver.gemspec
+++ b/nickserver.gemspec
@@ -19,7 +19,6 @@ Gem::Specification.new do |gem|
 
   gem.add_development_dependency 'rake'
   gem.add_development_dependency 'minitest'
-  gem.add_development_dependency 'webmock'
 
   gem.add_dependency 'reel'
   gem.add_dependency 'http'
diff --git a/test/Readme.md b/test/Readme.md
new file mode 100644
index 0000000..d9a3de8
--- /dev/null
+++ b/test/Readme.md
@@ -0,0 +1,34 @@
+Testing Strategy
+================
+
+Dispatcher Integration Tests
+----------------------------
+
+The dispatcher hands the requests to different handlers and responds with the
+first response it gets. We test the integration between the dispatcher and the
+handlers. We do so by confirming that a given query is handed to the right
+source in the expected manner.
+In order to do so we mock the sources. We also keep the server out of the loop.
+This way these tests should be deterministic and fast.
+
+Remote Tests
+------------
+
+Test the interaction of our sources with remote services. These tests make
+use of the real network. Therefore they are prone to network errors and non-
+deterministic server responses. With the expected result they will pass. Known
+failure cases should be covered in a unit test and lead to skipping the remote
+test. Unexpected remote behaviour should cause an Error. If you observe such an
+error:
+ * create a unit test for the source that triggers the same behaviour
+ * handle it appropriately in the source
+ * change the integration test to skip if the same behaviour happens again.
+
+
+Source Unit Tests
+-----------------
+
+As described above these should cover all possible network issues and make sure
+we return the right response in these cases.
+We can trigger the observed remote behaviour by mocking the adapter and thus
+make it deterministic.
diff --git a/test/integration/hkp_test.rb b/test/integration/hkp_test.rb
index bf78bb3..7d4bb6b 100644
--- a/test/integration/hkp_test.rb
+++ b/test/integration/hkp_test.rb
@@ -1,7 +1,9 @@
 require 'test_helper'
+require 'support/http_stub_helper'
 require 'nickserver/hkp/source'
 
 class HkpTest < Minitest::Test
+  include HttpStubHelper
 
   def setup
     super
@@ -39,26 +41,31 @@ class HkpTest < Minitest::Test
   end
 
   def test_key_info_not_found
-    uid = 'leaping_lemur@leap.se'
-    stub_sks_vindex_reponse(uid, status: 404)
-    assert_response_status_for_uid uid, 404
+    stubbing_http do
+      uid = 'leaping_lemur@leap.se'
+      stub_sks_vindex_reponse(uid, status: 404)
+      assert_response_status_for_uid uid, 404
+    end
   end
 
   def test_no_matching_key_found
-    uid = 'leaping_lemur@leap.se'
-    stub_sks_vindex_reponse(uid, status: 200)
-    assert_response_status_for_uid uid, 404
+    stubbing_http do
+      uid = 'leaping_lemur@leap.se'
+      stub_sks_vindex_reponse(uid, status: 200)
+      assert_response_status_for_uid uid, 404
+    end
   end
 
   def test_fetch_key
     uid    = 'cloudadmin@leap.se'
     key_id = 'E818C478D3141282F7590D29D041EB11B1647490'
-    stub_sks_vindex_reponse(uid, body: file_content(:leap_vindex_result))
-    stub_sks_get_reponse(key_id, body: file_content(:leap_public_key))
-
-    assert_response_for_uid(uid) do |response|
-      content = JSON.parse response.content
-      assert_equal file_content(:leap_public_key), content['openpgp']
+    stubbing_http do
+      stub_sks_vindex_reponse(uid, body: file_content(:leap_vindex_result))
+      stub_sks_get_reponse(key_id, body: file_content(:leap_public_key))
+      assert_response_for_uid(uid) do |response|
+        content = JSON.parse response.content
+        assert_equal file_content(:leap_public_key), content['openpgp']
+      end
     end
   end
 
@@ -66,50 +73,19 @@ class HkpTest < Minitest::Test
     uid    = 'cloudadmin@leap.se'
     key_id = 'E818C478D3141282F7590D29D041EB11B1647490'
 
-    stub_sks_vindex_reponse(uid, body: file_content(:leap_vindex_result))
-    stub_sks_get_reponse(key_id, status: 404)
-
-    assert_response_status_for_uid uid, 404
+    stubbing_http do
+      stub_sks_vindex_reponse(uid, body: file_content(:leap_vindex_result))
+      stub_sks_get_reponse(key_id, status: 404)
+      assert_response_status_for_uid uid, 404
+    end
   end
 
   def test_fetch_key_too_short
     uid    = 'chiiph@leap.se'
 
-    stub_sks_vindex_reponse(uid, body: file_content(:short_key_vindex_result))
-    assert_response_status_for_uid uid, 500
-  end
-
-  #
-  # real network tests
-  # remember: must be run with REAL_NET=true
-  #
-
-  def test_key_info_real_network
-    real_network do
-      uid = 'elijah@riseup.net'
-      assert_key_info_for_uid uid do |keys|
-        assert_equal 1, keys.size
-        assert keys.first.keyid =~ /00440025$/
-      end
-    end
-  end
-
-  def test_tls_validation_with_real_network
-    hkp_url = 'https://keys.mayfirst.org/pks/lookup'
-    ca_file = file_path('mayfirst-ca.pem')
-
-    real_network do
-      config.stub(:hkp_url, hkp_url) do
-        config.stub(:hkp_ca_file, ca_file) do
-        #config.stub(:hkp_ca_file, file_path('autistici-ca.pem')) do
-          assert File.exist?(Nickserver::Config.hkp_ca_file)
-          uid = 'elijah@riseup.net'
-          assert_key_info_for_uid uid do |keys|
-            assert_equal 1, keys.size
-            assert keys.first.keyid =~ /00440025$/
-          end
-        end
-      end
+    stubbing_http do
+      stub_sks_vindex_reponse(uid, body: file_content(:short_key_vindex_result))
+      assert_response_status_for_uid uid, 500
     end
   end
 
@@ -134,13 +110,11 @@ class HkpTest < Minitest::Test
     end
   end
 
-  def adapter
-    Nickserver::Adapters::CelluloidHttp.new
-  end
-
   def fetch_key_info(body_source, uid, &block)
-    stub_sks_vindex_reponse(uid, body: file_content(body_source))
-    assert_key_info_for_uid(uid, &block)
+    stubbing_http do
+      stub_sks_vindex_reponse uid, body: file_content(body_source)
+      assert_key_info_for_uid(uid, &block)
+    end
   end
 
 end
diff --git a/test/integration/nickserver_test.rb b/test/integration/nickserver_test.rb
index bdba6b7..e367e06 100644
--- a/test/integration/nickserver_test.rb
+++ b/test/integration/nickserver_test.rb
@@ -1,21 +1,20 @@
 require 'test_helper'
+require 'support/http_stub_helper'
 require 'nickserver/server'
 require 'json'
 
 #
 # Some important notes to understanding these tests:
 #
-# (1) Requests to 127.0.0.1 always bypass HTTP stub.
+# (1) We mock the http adapter. So no network is required.
 #
-# (2) All requests to nickserver are to 127.0.0.1.
+# (2) We actually start the nickserver on 127.0.0.1 and talk to it via http.
 #
 # (3) the "Host" header for requests to nickserver must be set (or Config.domain set)
 #
-# (4) When stubbing requests to couchdb, the couchdb host is changed from the
-# default (127.0.0.1) to a dummy value (notlocalhost).
-#
 
 class NickserverTest < Minitest::Test
+  include HttpStubHelper
 
   def setup
     super
@@ -107,7 +106,9 @@ class NickserverTest < Minitest::Test
   #
   def start(timeout = 1)
     server = Nickserver::ReelServer.new '127.0.0.1', config.port
-    yield server
+    stubbing_http do
+      yield server
+    end
   ensure
     server.terminate if server && server.alive?
   end
diff --git a/test/remote/celluloid_http_test.rb b/test/remote/celluloid_http_test.rb
new file mode 100644
index 0000000..46a5259
--- /dev/null
+++ b/test/remote/celluloid_http_test.rb
@@ -0,0 +1,27 @@
+require 'test_helper'
+require 'nickserver/adapters/celluloid_http'
+
+class Nickserver::Adapters::CelluloidHttpTest < Minitest::Test
+
+  def setup
+    super
+    Celluloid.boot
+  end
+
+  def teardown
+    Celluloid.shutdown
+    super
+  end
+
+  def test_https_for_hkp
+    url = Nickserver::Config.hkp_url
+    status, _body = adapter.get url
+    assert_equal 404, status
+  end
+
+  protected
+
+  def adapter
+    @adapter ||= Nickserver::Adapters::CelluloidHttp.new
+  end
+end
diff --git a/test/remote/hkp_source_test.rb b/test/remote/hkp_source_test.rb
new file mode 100644
index 0000000..aabc4d3
--- /dev/null
+++ b/test/remote/hkp_source_test.rb
@@ -0,0 +1,49 @@
+require 'test_helper'
+require 'nickserver/hkp/source'
+
+class RemoteHkpSourceTest < Minitest::Test
+
+  def setup
+    super
+    Celluloid.boot
+  end
+
+  def teardown
+    Celluloid.shutdown
+    super
+  end
+
+  def test_key_info
+    uid = 'elijah@riseup.net'
+    assert_key_info_for_uid uid do |keys|
+      assert_equal 1, keys.size
+      assert keys.first.keyid =~ /00440025$/
+    end
+  end
+
+  def test_tls_validation
+    hkp_url = 'https://keys.mayfirst.org/pks/lookup'
+    ca_file = file_path('mayfirst-ca.pem')
+
+    config.stub(:hkp_url, hkp_url) do
+      config.stub(:hkp_ca_file, ca_file) do
+      #config.stub(:hkp_ca_file, file_path('autistici-ca.pem')) do
+        assert File.exist?(Nickserver::Config.hkp_ca_file)
+        uid = 'elijah@riseup.net'
+        assert_key_info_for_uid uid do |keys|
+          assert_equal 1, keys.size
+          assert keys.first.keyid =~ /00440025$/
+        end
+      end
+    end
+  end
+
+  protected
+
+  def assert_key_info_for_uid(uid, &block)
+    Nickserver::Hkp::Source.new.search uid do |status, keys|
+      assert_equal 200, status
+      yield keys
+    end
+  end
+end
diff --git a/test/remote/nicknym_source_test.rb b/test/remote/nicknym_source_test.rb
index 0c6df65..e516895 100644
--- a/test/remote/nicknym_source_test.rb
+++ b/test/remote/nicknym_source_test.rb
@@ -14,9 +14,7 @@ class RemoteNicknymSourceTest < Minitest::Test
   end
 
   def test_truth
-    real_network do
-      assert source.available_for? 'mail.bitmask.net'
-    end
+    assert source.available_for? 'mail.bitmask.net'
   end
 
   protected
diff --git a/test/support/http_stub_helper.rb b/test/support/http_stub_helper.rb
new file mode 100644
index 0000000..6b05f98
--- /dev/null
+++ b/test/support/http_stub_helper.rb
@@ -0,0 +1,38 @@
+module HttpStubHelper
+
+  def stubbing_http
+    Nickserver::Adapters::CelluloidHttp.stub :new, adapter do
+      yield
+    end
+    adapter.verify
+  end
+
+  def stub_sks_vindex_reponse(uid, response = {})
+    stub_http_request :get, config.hkp_url,
+      query: {op: 'vindex', search: uid, exact: 'on', options: 'mr', fingerprint: 'on'},
+      response: response
+  end
+
+  def stub_sks_get_reponse(key_id, response = {})
+    stub_http_request :get, config.hkp_url,
+      query: {op: 'get', search: "0x"+key_id, exact: 'on', options: 'mr'},
+      response: response
+  end
+
+  def stub_couch_response(uid, response = {})
+    query = "\?key=#{"%22#{uid}%22"}&reduce=false"
+    stub_http_request :get,
+      /#{Regexp.escape(config.couch_url)}.*#{query}/,
+      response: response
+  end
+
+  def stub_http_request(verb, url, options = {})
+    response = {status: 200, body: ""}.merge(options.delete(:response) || {})
+    adapter.expect :get, [response[:status], response[:body]],
+      [url, options]
+  end
+
+  def adapter
+    @adapter ||= MiniTest::Mock.new
+  end
+end
diff --git a/test/test_helper.rb b/test/test_helper.rb
index 9f2c581..6bf3854 100644
--- a/test/test_helper.rb
+++ b/test/test_helper.rb
@@ -4,9 +4,6 @@ require 'rubygems'
 require 'kernel_ext'
 require 'bundler/setup'
 require 'minitest/autorun'
-silence_warnings do
-  require 'webmock/minitest'
-end
 require 'celluloid/test'
 require 'minitest/pride'
 require 'minitest/hell'
@@ -20,9 +17,6 @@ class Minitest::Test
 
   def setup
     Nickserver::Config.load
-
-    # by default, mock all non-localhost network connections
-    WebMock.disable_net_connect!(allow_localhost: true)
   end
 
   def file_content(filename)
@@ -33,38 +27,6 @@ class Minitest::Test
     "%s/files/%s" % [File.dirname(__FILE__), filename]
   end
 
-  def real_network
-    unless ENV['ONLY_LOCAL'] == 'true'
-      WebMock.allow_net_connect!
-      yield
-      WebMock.disable_net_connect!
-    end
-  end
-
-  def stub_sks_vindex_reponse(uid, opts = {})
-    options = {status: 200, body: ""}.merge(opts)
-    stub_http_request(:get, Nickserver::Config.hkp_url).with(
-      query: {op: 'vindex', search: uid, exact: 'on', options: 'mr', fingerprint: 'on'}
-    ).to_return(options)
-  end
-
-  def stub_sks_get_reponse(key_id, opts = {})
-    options = {status: 200, body: ""}.merge(opts)
-    stub_http_request(:get, config.hkp_url).with(
-      query: {op: 'get', search: "0x"+key_id, exact: 'on', options: 'mr'}
-    ).to_return(options)
-  end
-
-  def stub_couch_response(uid, opts = {})
-    # can't stub localhost, so set couch_host to anything else
-    config.stub :couch_host, 'notlocalhost' do
-      options = {status: 200, body: ""}.merge(opts)
-      query = "\?key=#{"%22#{uid}%22"}&reduce=false"
-      stub_http_request(:get, /#{Regexp.escape(config.couch_url)}.*#{query}/).to_return(options)
-      yield
-    end
-  end
-
   def config
     Nickserver::Config
   end
diff --git a/test/unit/adapters/celluloid_http_test.rb b/test/unit/adapters/celluloid_http_test.rb
deleted file mode 100644
index 68b9606..0000000
--- a/test/unit/adapters/celluloid_http_test.rb
+++ /dev/null
@@ -1,48 +0,0 @@
-require 'test_helper'
-require 'nickserver/adapters/celluloid_http'
-
-class Nickserver::Adapters::CelluloidHttpTest < Minitest::Test
-
-  def setup
-    super
-    Celluloid.boot
-  end
-
-  def teardown
-    Celluloid.shutdown
-    super
-  end
-
-  def test_request_without_query
-    url = 'http://url.to'
-    stub_http_request(:get, url)
-      .to_return status: 200, body: 'body'
-    status, body = adapter.get url
-    assert_equal 200, status
-    assert_equal 'body', body
-  end
-
-  def test_successful_request_with_query
-    url = 'http://url.to'
-    stub_http_request(:get, url)
-      .with(query: {key: :value})
-      .to_return status: 200, body: 'body'
-    status, body = adapter.get(url, query: {key: :value})
-    assert_equal 200, status
-    assert_equal 'body', body
-  end
-
-  def test_https_for_hkp
-    url = Nickserver::Config.hkp_url
-    real_network do
-      status, _body = adapter.get url
-      assert_equal 404, status
-    end
-  end
-
-  protected
-
-  def adapter
-    @adapter ||= Nickserver::Adapters::CelluloidHttp.new
-  end
-end