From ec875169b0231d84bb8c55bbe91c52b896561f1e Mon Sep 17 00:00:00 2001 From: Azul Date: Mon, 12 Sep 2016 16:34:25 +0200 Subject: 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. --- lib/nickserver/nicknym/source.rb | 2 +- lib/nickserver/reel_server.rb | 1 + nickserver.gemspec | 1 - test/Readme.md | 34 ++++++++++++ test/integration/hkp_test.rb | 88 +++++++++++-------------------- test/integration/nickserver_test.rb | 13 ++--- test/remote/celluloid_http_test.rb | 27 ++++++++++ test/remote/hkp_source_test.rb | 49 +++++++++++++++++ test/remote/nicknym_source_test.rb | 4 +- test/support/http_stub_helper.rb | 38 +++++++++++++ test/test_helper.rb | 38 ------------- test/unit/adapters/celluloid_http_test.rb | 48 ----------------- 12 files changed, 189 insertions(+), 154 deletions(-) create mode 100644 test/Readme.md create mode 100644 test/remote/celluloid_http_test.rb create mode 100644 test/remote/hkp_source_test.rb create mode 100644 test/support/http_stub_helper.rb delete mode 100644 test/unit/adapters/celluloid_http_test.rb 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 -- cgit v1.2.3