From d49dbea922d37329482a846f39c51bcaaa0ebc3c Mon Sep 17 00:00:00 2001 From: Artem Date: Thu, 19 Jul 2018 22:01:44 +1000 Subject: [PATCH 01/27] Add empty get() test --- test/helpers/test_collection.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/helpers/test_collection.rb b/test/helpers/test_collection.rb index dd7da10b14..55c954ce8f 100644 --- a/test/helpers/test_collection.rb +++ b/test/helpers/test_collection.rb @@ -31,6 +31,10 @@ def test_enumerable assert_respond_to @subject, :each end + def test_nil_get + assert_nil @subject.get(nil) + end + def teardown @factory.cleanup end From 77125ea986a054784bd1fb046ffee7068d85252e Mon Sep 17 00:00:00 2001 From: Artem Date: Fri, 20 Jul 2018 14:36:11 +1000 Subject: [PATCH 02/27] Add Pubsub to fog bin --- lib/fog/bin/google.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/fog/bin/google.rb b/lib/fog/bin/google.rb index 23dcf1d005..5c2b47513b 100644 --- a/lib/fog/bin/google.rb +++ b/lib/fog/bin/google.rb @@ -15,6 +15,8 @@ def class_for(key) Fog::Storage::Google when :sql Fog::Google::SQL + when :pubsub + Fog::Google::Pubsub else raise ArgumentError, "Unsupported #{self} service: #{key}" end @@ -33,6 +35,8 @@ def [](service) Fog::Google::Monitoring.new when :sql Fog::Google::SQL.new + when :pubsub + Fog::Google::Pubsub.new when :storage Fog::Logger.warning("Google[:storage] is not recommended, use Storage[:google] for portability") Fog::Storage.new(:provider => "Google") From aaa65b2737689a547deebe3356222a552ee46bb6 Mon Sep 17 00:00:00 2001 From: Artem Date: Fri, 20 Jul 2018 15:04:01 +1000 Subject: [PATCH 03/27] Add more unit tests + test fixtures for #352 --- test/unit/compute/test_common_collections.rb | 16 +++++-- test/unit/compute/test_common_models.rb | 4 +- test/unit/dns/test_common_collections.rb | 40 +++++++++++++++++ .../unit/monitoring/test_comon_collections.rb | 40 +++++++++++++++++ test/unit/pubsub/test_common_collections.rb | 38 ++++++++++++++++ test/unit/sql/test_common_collections.rb | 43 +++++++++++++++++++ .../storage/test_common_json_collections.rb | 38 ++++++++++++++++ .../storage/test_common_xml_collections.rb | 40 +++++++++++++++++ 8 files changed, 253 insertions(+), 6 deletions(-) create mode 100644 test/unit/dns/test_common_collections.rb create mode 100644 test/unit/monitoring/test_comon_collections.rb create mode 100644 test/unit/pubsub/test_common_collections.rb create mode 100644 test/unit/sql/test_common_collections.rb create mode 100644 test/unit/storage/test_common_json_collections.rb create mode 100644 test/unit/storage/test_common_xml_collections.rb diff --git a/test/unit/compute/test_common_collections.rb b/test/unit/compute/test_common_collections.rb index a07e9e08b6..74e1d6e5ca 100644 --- a/test/unit/compute/test_common_collections.rb +++ b/test/unit/compute/test_common_collections.rb @@ -1,18 +1,18 @@ require "helpers/test_helper" +require "pry" class UnitTestCollections < MiniTest::Test def setup Fog.mock! + @client = Fog::Compute.new(:provider => "Google", :google_project => "foo") - # Top-level ancestors we do not dest - common_ancestors = [Fog::Collection, Fog::Association, Fog::PagedCollection] # Projects do not have a "list" method in compute API exceptions = [Fog::Compute::Google::Projects] # Enumerate all descendants of Fog::Collection descendants = ObjectSpace.each_object(Fog::Collection.singleton_class).to_a - @collections = descendants - common_ancestors - exceptions + @collections = descendants.select {|d| d.name.match /Fog::Compute::Google/ } - exceptions end def teardown @@ -28,4 +28,14 @@ def test_common_methods assert obj.respond_to?(:each), "#{klass} should behave like Enumerable" end end + + def test_collection_get_arguments + # TODO: Fixture for #352 + skip + @collections.each do |klass| + obj = klass.new + assert_operator(obj.method(:get).arity, :<=, 1, + "#{klass} should have at most 1 required parameter in get()") + end + end end diff --git a/test/unit/compute/test_common_models.rb b/test/unit/compute/test_common_models.rb index d830538475..f5fbc5afdc 100644 --- a/test/unit/compute/test_common_models.rb +++ b/test/unit/compute/test_common_models.rb @@ -5,8 +5,6 @@ def setup Fog.mock! @client = Fog::Compute.new(:provider => "Google", :google_project => "foo") - # Top-level ancestors we do not test - common_ancestors = [Fog::Model, Fog::Compute::Server] # Do not test models that do not have a create method in API exceptions = [ Fog::Compute::Google::MachineType, Fog::Compute::Google::Region, @@ -18,7 +16,7 @@ def setup # Enumerate all descendants of Fog::Model descendants = ObjectSpace.each_object(Fog::Model.singleton_class).to_a - @models = descendants - common_ancestors - exceptions + @models = descendants.select {|d| d.name.match /Fog::Compute::Google/ } - exceptions end def teardown diff --git a/test/unit/dns/test_common_collections.rb b/test/unit/dns/test_common_collections.rb new file mode 100644 index 0000000000..236d927747 --- /dev/null +++ b/test/unit/dns/test_common_collections.rb @@ -0,0 +1,40 @@ +require "helpers/test_helper" +require "pry" + +class UnitTestDNSCollections < MiniTest::Test + def setup + Fog.mock! + @client = Fog::DNS.new(provider: "google") + + # DNS Projects API does not support 'list', so 'all' method is not possible + exceptions = [Fog::DNS::Google::Projects] + # Enumerate all descendants of Fog::Collection + descendants = ObjectSpace.each_object(Fog::Collection.singleton_class) + + @collections = descendants.select { |x| x.name.match /Fog::DNS::Google/ } - exceptions + end + + def teardown + Fog.unmock! + end + + def test_common_methods + # This tests whether Fog::Compute::Google collections have common lifecycle methods + @collections.each do |klass| + obj = klass.new + assert obj.respond_to?(:all), "#{klass} should have an .all method" + assert obj.respond_to?(:get), "#{klass} should have a .get method" + assert obj.respond_to?(:each), "#{klass} should behave like Enumerable" + end + end + + def test_collection_get_arguments + # TODO: Fixture for #352 + skip + @collections.each do |klass| + obj = klass.new + assert_operator(obj.method(:get).arity, :<=, 1, + "#{klass} should have at most 1 required parameter in get()") + end + end +end diff --git a/test/unit/monitoring/test_comon_collections.rb b/test/unit/monitoring/test_comon_collections.rb new file mode 100644 index 0000000000..581a618000 --- /dev/null +++ b/test/unit/monitoring/test_comon_collections.rb @@ -0,0 +1,40 @@ +require "helpers/test_helper" +require "pry" + +class UnitTestMonitoringCollections < MiniTest::Test + def setup + Fog.mock! + @client = Fog::Monitoring.new(provider: "google") + + # TimeSeries API has no 'get' method + exceptions = [Fog::Google::Monitoring::TimeseriesCollection] + # Enumerate all descendants of Fog::Collection + descendants = ObjectSpace.each_object(Fog::Collection.singleton_class).to_a + + @collections = descendants.select { |x| x.name.match /Fog::Google::Monitoring/ } - exceptions + end + + def teardown + Fog.unmock! + end + + def test_common_methods + # This tests whether Fog::Compute::Google collections have common lifecycle methods + @collections.each do |klass| + obj = klass.new + assert obj.respond_to?(:all), "#{klass} should have an .all method" + assert obj.respond_to?(:get), "#{klass} should have a .get method" + assert obj.respond_to?(:each), "#{klass} should behave like Enumerable" + end + end + + def test_collection_get_arguments + # TODO: Fixture for #352 + skip + @collections.each do |klass| + obj = klass.new + assert_operator(obj.method(:get).arity, :<=, 1, + "#{klass} should have at most 1 required parameter in get()") + end + end +end diff --git a/test/unit/pubsub/test_common_collections.rb b/test/unit/pubsub/test_common_collections.rb new file mode 100644 index 0000000000..071c354b55 --- /dev/null +++ b/test/unit/pubsub/test_common_collections.rb @@ -0,0 +1,38 @@ +require "helpers/test_helper" +require "pry" + +class UnitTestPubsubCollections < MiniTest::Test + def setup + Fog.mock! + @client = Fog::Google::Pubsub.new + + exceptions = [] + # Enumerate all descendants of Fog::Collection + descendants = ObjectSpace.each_object(Fog::Collection.singleton_class) + + @collections = descendants.select { |x| x.name.match /Fog::Google::Pubsub/ } - exceptions + end + + def teardown + Fog.unmock! + end + + def test_common_methods + # This tests whether Fog::Compute::Google collections have common lifecycle methods + @collections.each do |klass| + obj = klass.new + assert obj.respond_to?(:all), "#{klass} should have an .all method" + assert obj.respond_to?(:get), "#{klass} should have a .get method" + assert obj.respond_to?(:each), "#{klass} should behave like Enumerable" + end + end + + def test_collection_get_arguments + # TODO: Fixture for #352 + @collections.each do |klass| + obj = klass.new + assert_operator(obj.method(:get).arity, :<=, 1, + "#{klass} should have at most 1 required parameter in get()") + end + end +end diff --git a/test/unit/sql/test_common_collections.rb b/test/unit/sql/test_common_collections.rb new file mode 100644 index 0000000000..7326b732c7 --- /dev/null +++ b/test/unit/sql/test_common_collections.rb @@ -0,0 +1,43 @@ +require "helpers/test_helper" +require "pry" + +class UnitTestSQLCollections < MiniTest::Test + def setup + Fog.mock! + @client = Fog::Google::SQL.new + + # SQL Users API doesn't have a get method + # SQL Flags API has only a 'list' method + exceptions = [Fog::Google::SQL::Users, + Fog::Google::SQL::Tiers, + Fog::Google::SQL::Flags] + # Enumerate all descendants of Fog::Collection + descendants = ObjectSpace.each_object(Fog::Collection.singleton_class) + + @collections = descendants.select { |x| x.name.match /Fog::Google::SQL/ } - exceptions + end + + def teardown + Fog.unmock! + end + + def test_common_methods + # This tests whether Fog::Compute::Google collections have common lifecycle methods + @collections.each do |klass| + obj = klass.new + assert obj.respond_to?(:all), "#{klass} should have an .all method" + assert obj.respond_to?(:get), "#{klass} should have a .get method" + assert obj.respond_to?(:each), "#{klass} should behave like Enumerable" + end + end + + def test_collection_get_arguments + # TODO: Fixture for #352 + skip + @collections.each do |klass| + obj = klass.new + assert_operator(obj.method(:get).arity, :<=, 1, + "#{klass} should have at most 1 required parameter in get()") + end + end +end diff --git a/test/unit/storage/test_common_json_collections.rb b/test/unit/storage/test_common_json_collections.rb new file mode 100644 index 0000000000..567ea375d7 --- /dev/null +++ b/test/unit/storage/test_common_json_collections.rb @@ -0,0 +1,38 @@ +require "helpers/test_helper" +require "pry" + +class UnitTestStorageJSONCollections < MiniTest::Test + def setup + Fog.mock! + @client = Fog::Storage.new(provider: "google") + + # Enumerate all descendants of Fog::Collection + descendants = ObjectSpace.each_object(Fog::Collection.singleton_class) + + @collections = descendants.select { + |x| x.name.match /Fog::Storage::GoogleJSON/ + } + end + + def teardown + Fog.unmock! + end + + def test_common_methods + # This tests whether Fog::Compute::Google collections have common lifecycle methods + @collections.each do |klass| + obj = klass.new + assert obj.respond_to?(:all), "#{klass} should have an .all method" + assert obj.respond_to?(:get), "#{klass} should have a .get method" + assert obj.respond_to?(:each), "#{klass} should behave like Enumerable" + end + end + + def test_collection_get_arguments + @collections.each do |klass| + obj = klass.new + assert_operator(obj.method(:get).arity, :<=, 1, + "#{klass} should have at most 1 required parameter in get()") + end + end +end diff --git a/test/unit/storage/test_common_xml_collections.rb b/test/unit/storage/test_common_xml_collections.rb new file mode 100644 index 0000000000..94a0db2105 --- /dev/null +++ b/test/unit/storage/test_common_xml_collections.rb @@ -0,0 +1,40 @@ +require "helpers/test_helper" +require "pry" + +class UnitTestStorageXMLCollections < MiniTest::Test + def setup + Fog.mock! + @client = Fog::Storage.new(provider: "google", + google_storage_access_key_id: "", + google_storage_secret_access_key: "") + + # Enumerate all descendants of Fog::Collection + descendants = ObjectSpace.each_object(Fog::Collection.singleton_class) + + @collections = descendants.select { + |x| x.name.match /Fog::Storage::GoogleXML/ + } + end + + def teardown + Fog.unmock! + end + + def test_common_methods + # This tests whether Fog::Compute::Google collections have common lifecycle methods + @collections.each do |klass| + obj = klass.new + assert obj.respond_to?(:all), "#{klass} should have an .all method" + assert obj.respond_to?(:get), "#{klass} should have a .get method" + assert obj.respond_to?(:each), "#{klass} should behave like Enumerable" + end + end + + def test_collection_get_arguments + @collections.each do |klass| + obj = klass.new + assert_operator(obj.method(:get).arity, :<=, 1, + "#{klass} should have at most 1 required parameter in get()") + end + end +end From b7d05006b519e31149d7be5fb2a216547f14ba49 Mon Sep 17 00:00:00 2001 From: Artem Date: Fri, 20 Jul 2018 20:36:36 +1000 Subject: [PATCH 04/27] Fixup nil_get_test - Enable for get methods with multiple parameters - Mark as skip by default since #33 causes to leak info --- test/helpers/test_collection.rb | 10 +++++++++- test/unit/compute/test_common_collections.rb | 2 +- test/unit/compute/test_common_models.rb | 16 ++++++++-------- test/unit/dns/test_common_collections.rb | 2 +- test/unit/monitoring/test_comon_collections.rb | 2 +- test/unit/pubsub/test_common_collections.rb | 2 +- test/unit/sql/test_common_collections.rb | 2 +- .../unit/storage/test_common_json_collections.rb | 6 +++--- test/unit/storage/test_common_xml_collections.rb | 6 +++--- 9 files changed, 28 insertions(+), 20 deletions(-) diff --git a/test/helpers/test_collection.rb b/test/helpers/test_collection.rb index 55c954ce8f..7bf4c99683 100644 --- a/test/helpers/test_collection.rb +++ b/test/helpers/test_collection.rb @@ -32,7 +32,15 @@ def test_enumerable end def test_nil_get - assert_nil @subject.get(nil) + # Fixture for #33 + skip + if @subject.method(:get).arity <= 1 + assert_nil @subject.get(nil) + elsif @subject.method(:get).arity == 2 + assert_nil @subject.get(nil) + else + fail "Unexpected number of required get parameters" + end end def teardown diff --git a/test/unit/compute/test_common_collections.rb b/test/unit/compute/test_common_collections.rb index 74e1d6e5ca..6a20b4ffa4 100644 --- a/test/unit/compute/test_common_collections.rb +++ b/test/unit/compute/test_common_collections.rb @@ -12,7 +12,7 @@ def setup # Enumerate all descendants of Fog::Collection descendants = ObjectSpace.each_object(Fog::Collection.singleton_class).to_a - @collections = descendants.select {|d| d.name.match /Fog::Compute::Google/ } - exceptions + @collections = descendants.select { |d| d.name.match /Fog::Compute::Google/ } - exceptions end def teardown diff --git a/test/unit/compute/test_common_models.rb b/test/unit/compute/test_common_models.rb index f5fbc5afdc..a3b2cbf907 100644 --- a/test/unit/compute/test_common_models.rb +++ b/test/unit/compute/test_common_models.rb @@ -6,17 +6,17 @@ def setup @client = Fog::Compute.new(:provider => "Google", :google_project => "foo") # Do not test models that do not have a create method in API - exceptions = [ Fog::Compute::Google::MachineType, - Fog::Compute::Google::Region, - Fog::Compute::Google::DiskType, - Fog::Compute::Google::Operation, - Fog::Compute::Google::Zone, - Fog::Compute::Google::Snapshot, - Fog::Compute::Google::Project ] + exceptions = [Fog::Compute::Google::MachineType, + Fog::Compute::Google::Region, + Fog::Compute::Google::DiskType, + Fog::Compute::Google::Operation, + Fog::Compute::Google::Zone, + Fog::Compute::Google::Snapshot, + Fog::Compute::Google::Project] # Enumerate all descendants of Fog::Model descendants = ObjectSpace.each_object(Fog::Model.singleton_class).to_a - @models = descendants.select {|d| d.name.match /Fog::Compute::Google/ } - exceptions + @models = descendants.select { |d| d.name.match /Fog::Compute::Google/ } - exceptions end def teardown diff --git a/test/unit/dns/test_common_collections.rb b/test/unit/dns/test_common_collections.rb index 236d927747..d0f2a15f35 100644 --- a/test/unit/dns/test_common_collections.rb +++ b/test/unit/dns/test_common_collections.rb @@ -11,7 +11,7 @@ def setup # Enumerate all descendants of Fog::Collection descendants = ObjectSpace.each_object(Fog::Collection.singleton_class) - @collections = descendants.select { |x| x.name.match /Fog::DNS::Google/ } - exceptions + @collections = descendants.select { |d| d.name.match /Fog::DNS::Google/ } - exceptions end def teardown diff --git a/test/unit/monitoring/test_comon_collections.rb b/test/unit/monitoring/test_comon_collections.rb index 581a618000..0ce3f5b754 100644 --- a/test/unit/monitoring/test_comon_collections.rb +++ b/test/unit/monitoring/test_comon_collections.rb @@ -11,7 +11,7 @@ def setup # Enumerate all descendants of Fog::Collection descendants = ObjectSpace.each_object(Fog::Collection.singleton_class).to_a - @collections = descendants.select { |x| x.name.match /Fog::Google::Monitoring/ } - exceptions + @collections = descendants.select { |d| d.name.match /Fog::Google::Monitoring/ } - exceptions end def teardown diff --git a/test/unit/pubsub/test_common_collections.rb b/test/unit/pubsub/test_common_collections.rb index 071c354b55..aea39e2d28 100644 --- a/test/unit/pubsub/test_common_collections.rb +++ b/test/unit/pubsub/test_common_collections.rb @@ -10,7 +10,7 @@ def setup # Enumerate all descendants of Fog::Collection descendants = ObjectSpace.each_object(Fog::Collection.singleton_class) - @collections = descendants.select { |x| x.name.match /Fog::Google::Pubsub/ } - exceptions + @collections = descendants.select { |d| d.name.match /Fog::Google::Pubsub/ } - exceptions end def teardown diff --git a/test/unit/sql/test_common_collections.rb b/test/unit/sql/test_common_collections.rb index 7326b732c7..b3429d97d6 100644 --- a/test/unit/sql/test_common_collections.rb +++ b/test/unit/sql/test_common_collections.rb @@ -14,7 +14,7 @@ def setup # Enumerate all descendants of Fog::Collection descendants = ObjectSpace.each_object(Fog::Collection.singleton_class) - @collections = descendants.select { |x| x.name.match /Fog::Google::SQL/ } - exceptions + @collections = descendants.select { |d| d.name.match /Fog::Google::SQL/ } - exceptions end def teardown diff --git a/test/unit/storage/test_common_json_collections.rb b/test/unit/storage/test_common_json_collections.rb index 567ea375d7..2a71d498a7 100644 --- a/test/unit/storage/test_common_json_collections.rb +++ b/test/unit/storage/test_common_json_collections.rb @@ -9,9 +9,9 @@ def setup # Enumerate all descendants of Fog::Collection descendants = ObjectSpace.each_object(Fog::Collection.singleton_class) - @collections = descendants.select { - |x| x.name.match /Fog::Storage::GoogleJSON/ - } + @collections = descendants.select do |d| + d.name.match /Fog::Storage::GoogleJSON/ + end end def teardown diff --git a/test/unit/storage/test_common_xml_collections.rb b/test/unit/storage/test_common_xml_collections.rb index 94a0db2105..95c75f0eee 100644 --- a/test/unit/storage/test_common_xml_collections.rb +++ b/test/unit/storage/test_common_xml_collections.rb @@ -11,9 +11,9 @@ def setup # Enumerate all descendants of Fog::Collection descendants = ObjectSpace.each_object(Fog::Collection.singleton_class) - @collections = descendants.select { - |x| x.name.match /Fog::Storage::GoogleXML/ - } + @collections = descendants.select do |d| + d.name.match /Fog::Storage::GoogleXML/ + end end def teardown From 30f68c1f36584cf237250ead2cde92b3fb8a1089 Mon Sep 17 00:00:00 2001 From: Artem Date: Sat, 21 Jul 2018 20:59:26 +1000 Subject: [PATCH 05/27] Small drive-by doc updates --- CONTRIBUTING.md | 16 +++++++++++----- README.md | 2 -- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 970822bb30..ef45ddd974 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -101,20 +101,26 @@ test: Then you can run all the live tests: ```shell -$ rake test +$ bundle exec rake test ``` or just one API: ``` -$ rake test:compute +$ bundle exec rake test:compute ``` (See `rake -T` for all available tasks) -or just one file: +or just one test: ```shell -$ rake test TEST=test/integration/compute/test_servers.rb TESTOPTS="--name=TestServers#test_bootstrap_ssh_destroy" +$ bundle exec rake test TESTOPTS="--name=TestServers#test_bootstrap_ssh_destroy" +``` + +or a series of tests by name: + +``` +$ bundle exec rake test TESTOPTS="--name=/test_nil_get/" ``` #### Unit tests @@ -133,7 +139,7 @@ We're in progress of extending the library with more unit tests and contribution #### The transition from `shindo` to Minitest Previously, [shindo](https://github.com/geemus/shindo) was the primary testing framework. -We've started moving away from it, and to Minitest, but some artifacts may remain. +We've moved away from it, and to Minitest, but some artifacts may remain. For more information on transition, read [#50](https://github.com/fog/fog-google/issues/50). diff --git a/README.md b/README.md index 528cdf11f3..c1580ac663 100644 --- a/README.md +++ b/README.md @@ -42,8 +42,6 @@ We are always looking for people to improve our code and test coverage, so pleas ## Pubsub -Note: You **must** have a version of google-api-client > 0.8.5 to use the Pub/Sub API; previous versions will not work. - Fog mostly implements [v1](https://cloud.google.com/pubsub/docs/reference/rest/) of the Google Cloud Pub/Sub API; however some less common API methods are missing. Pull requests for additions would be greatly appreciated. ## Installation From d662f7e8ab873027097576c14e6325887d3b0138 Mon Sep 17 00:00:00 2001 From: Artem Date: Sun, 22 Jul 2018 13:00:49 +1000 Subject: [PATCH 06/27] Fix get() logic for instance_id `nil` --- lib/fog/google/models/sql/instances.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/fog/google/models/sql/instances.rb b/lib/fog/google/models/sql/instances.rb index b608af0136..5b4c15f48f 100644 --- a/lib/fog/google/models/sql/instances.rb +++ b/lib/fog/google/models/sql/instances.rb @@ -23,7 +23,10 @@ def all # @return [Fog::Google::SQL::Instance] Instance resource def get(instance_id) instance = service.get_instance(instance_id).to_h - if instance + # XXX if we pass `nil` to get() it returns empty DB object with + # kind set to "sql#instancesList" + # see https://github.com/google/google-api-ruby-client/issues/699 + if instance[:kind].eql?("sql#instance") new(instance) end rescue ::Google::Apis::ClientError => e From 5dad4fecb7504b1da3c41221d784f94780492294 Mon Sep 17 00:00:00 2001 From: Artem Date: Sun, 22 Jul 2018 13:17:33 +1000 Subject: [PATCH 07/27] Fix logic to work with nil in get() Fixes the following error: Google::Apis::ClientError: invalid: Invalid value for field 'filter': 'name eq '. Invalid list filter expression. --- lib/fog/compute/google/models/forwarding_rules.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/fog/compute/google/models/forwarding_rules.rb b/lib/fog/compute/google/models/forwarding_rules.rb index 773fe8bcd9..bbc1e13b98 100644 --- a/lib/fog/compute/google/models/forwarding_rules.rb +++ b/lib/fog/compute/google/models/forwarding_rules.rb @@ -32,7 +32,7 @@ def get(identity, region = nil) response = nil if region response = service.get_forwarding_rule(identity, region).to_h - else + elsif identity response = all( :filter => "name eq #{identity}", :max_results => 1 ).first From 5a2cd80ac989bcdcf24370fc7401989e6da127f1 Mon Sep 17 00:00:00 2001 From: Artem Date: Sun, 22 Jul 2018 13:29:12 +1000 Subject: [PATCH 08/27] Fix nil get in servers and target_http_proxies See #33 --- lib/fog/compute/google/models/servers.rb | 2 +- lib/fog/compute/google/models/target_http_proxies.rb | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/fog/compute/google/models/servers.rb b/lib/fog/compute/google/models/servers.rb index 21b5ae9d60..53530748b3 100644 --- a/lib/fog/compute/google/models/servers.rb +++ b/lib/fog/compute/google/models/servers.rb @@ -31,7 +31,7 @@ def get(identity, zone = nil) response = nil if zone response = service.get_server(identity, zone).to_h - else + elseif identity server = all(:filter => "name eq .*#{identity}").first response = server.attributes if server end diff --git a/lib/fog/compute/google/models/target_http_proxies.rb b/lib/fog/compute/google/models/target_http_proxies.rb index 9c84473392..7af21c696e 100644 --- a/lib/fog/compute/google/models/target_http_proxies.rb +++ b/lib/fog/compute/google/models/target_http_proxies.rb @@ -10,8 +10,9 @@ def all(_filters = {}) end def get(identity) - if target_http_proxy = service.get_target_http_proxy(identity).to_h - new(target_http_proxy) + if identity + target_http_proxy = service.get_target_http_proxy(identity).to_h + new(target_http_proxy) if target_http_proxy end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 From 2e25d1c9db5dce289d2aca466f3d5c1c15c39f1e Mon Sep 17 00:00:00 2001 From: Artem Date: Sun, 22 Jul 2018 14:49:18 +1000 Subject: [PATCH 09/27] Fix nil_get test --- test/helpers/test_collection.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/helpers/test_collection.rb b/test/helpers/test_collection.rb index 7bf4c99683..f866a9fd20 100644 --- a/test/helpers/test_collection.rb +++ b/test/helpers/test_collection.rb @@ -37,7 +37,7 @@ def test_nil_get if @subject.method(:get).arity <= 1 assert_nil @subject.get(nil) elsif @subject.method(:get).arity == 2 - assert_nil @subject.get(nil) + assert_nil @subject.get(nil, nil) else fail "Unexpected number of required get parameters" end From 22effdf05c2b900e63fdcd8ca3154965f9e95ecd Mon Sep 17 00:00:00 2001 From: Artem Date: Sun, 22 Jul 2018 15:29:26 +1000 Subject: [PATCH 10/27] Refactor compute get methods This should fix both #33 and #352 for compute --- lib/fog/compute/google/models/addresses.rb | 12 +++++-- .../compute/google/models/backend_services.rb | 5 +-- lib/fog/compute/google/models/disk_types.rb | 11 +++--- lib/fog/compute/google/models/disks.rb | 14 +++++--- lib/fog/compute/google/models/firewalls.rb | 5 +-- .../compute/google/models/forwarding_rules.rb | 18 ++++------ .../compute/google/models/global_addresses.rb | 5 +-- .../google/models/global_forwarding_rules.rb | 5 +-- .../google/models/http_health_checks.rb | 7 ++-- lib/fog/compute/google/models/images.rb | 36 +++++++++++-------- .../google/models/instance_group_managers.rb | 12 ++++--- .../compute/google/models/instance_groups.rb | 18 +++++----- .../google/models/instance_templates.rb | 5 +-- .../compute/google/models/machine_types.rb | 17 ++++++--- lib/fog/compute/google/models/networks.rb | 5 +-- lib/fog/compute/google/models/operations.rb | 14 ++++---- lib/fog/compute/google/models/projects.rb | 5 +-- lib/fog/compute/google/models/regions.rb | 5 +-- lib/fog/compute/google/models/routes.rb | 5 +-- lib/fog/compute/google/models/servers.rb | 14 ++++---- lib/fog/compute/google/models/snapshots.rb | 9 ++--- .../compute/google/models/ssl_certificates.rb | 7 ++-- lib/fog/compute/google/models/subnetworks.rb | 12 +++++-- .../google/models/target_http_proxies.rb | 2 +- .../google/models/target_https_proxies.rb | 5 +-- .../compute/google/models/target_instances.rb | 16 ++++----- lib/fog/compute/google/models/target_pools.rb | 17 ++++----- lib/fog/compute/google/models/url_maps.rb | 5 +-- lib/fog/compute/google/models/zones.rb | 6 ++-- test/helpers/test_collection.rb | 2 -- test/unit/compute/test_common_collections.rb | 2 -- 31 files changed, 170 insertions(+), 131 deletions(-) diff --git a/lib/fog/compute/google/models/addresses.rb b/lib/fog/compute/google/models/addresses.rb index adb5470d72..dcbef112d2 100755 --- a/lib/fog/compute/google/models/addresses.rb +++ b/lib/fog/compute/google/models/addresses.rb @@ -23,9 +23,15 @@ def all(region: nil, filter: nil, max_results: nil, order_by: nil, page_token: n load(data.map(&:to_h)) end - def get(identity, region) - if address = service.get_address(identity, region).to_h - new(address) + def get(identity, region = nil) + if region + address = service.get_address(identity, region).to_h + return new(address) + elsif identity + response = all(:filter => "name eq #{identity}", + :max_results => 1) + address = response.first unless response.empty? + return address end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 diff --git a/lib/fog/compute/google/models/backend_services.rb b/lib/fog/compute/google/models/backend_services.rb index f55c44bd58..5e7d7d1335 100644 --- a/lib/fog/compute/google/models/backend_services.rb +++ b/lib/fog/compute/google/models/backend_services.rb @@ -10,8 +10,9 @@ def all(_filters = {}) end def get(identity) - if backend_service = service.get_backend_service(identity) - new(backend_service.to_h) + if identity + backend_service = service.get_backend_service(identity).to_h + return new(backend_service) end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 diff --git a/lib/fog/compute/google/models/disk_types.rb b/lib/fog/compute/google/models/disk_types.rb index cc81bd6509..6e424ed62e 100644 --- a/lib/fog/compute/google/models/disk_types.rb +++ b/lib/fog/compute/google/models/disk_types.rb @@ -24,15 +24,14 @@ def all(zone: nil, filter: nil, max_results: nil, end def get(identity, zone = nil) - response = nil if zone - response = service.get_disk_type(identity, zone).to_h + disk_type = service.get_disk_type(identity, zone).to_h + return new(disk_type) else - disk_types = all(:filter => "name eq .*#{identity}") - response = disk_types.first.attributes unless disk_types.empty? + response = all(:filter => "name eq .*#{identity}") + disk_type = response.first unless response.empty? + return disk_type end - return nil if response.nil? - new(response) rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 nil diff --git a/lib/fog/compute/google/models/disks.rb b/lib/fog/compute/google/models/disks.rb index 9588fdd99a..d7fe166e88 100644 --- a/lib/fog/compute/google/models/disks.rb +++ b/lib/fog/compute/google/models/disks.rb @@ -23,10 +23,16 @@ def all(zone: nil, filter: nil, max_results: nil, order_by: nil, load(data.map(&:to_h)) end - def get(identity, zone) - response = service.get_disk(identity, zone) - return nil if response.nil? - new(response.to_h) + def get(identity, zone = nil) + if zone + disk = service.get_disk(identity, zone).to_h + return new(disk) + elsif identity + response = all(:filter => "name eq #{identity}", + :max_results => 1) + disk = response.first unless response.empty? + return disk + end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 nil diff --git a/lib/fog/compute/google/models/firewalls.rb b/lib/fog/compute/google/models/firewalls.rb index 072002415d..854b4a1daa 100644 --- a/lib/fog/compute/google/models/firewalls.rb +++ b/lib/fog/compute/google/models/firewalls.rb @@ -10,8 +10,9 @@ def all(opts = {}) end def get(identity) - if firewall = service.get_firewall(identity) - new(firewall.to_h) + if identity + firewall = service.get_firewall(identity).to_h + return new(firewall) end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 diff --git a/lib/fog/compute/google/models/forwarding_rules.rb b/lib/fog/compute/google/models/forwarding_rules.rb index bbc1e13b98..46739b135a 100644 --- a/lib/fog/compute/google/models/forwarding_rules.rb +++ b/lib/fog/compute/google/models/forwarding_rules.rb @@ -4,9 +4,7 @@ class Google class ForwardingRules < Fog::Collection model Fog::Compute::Google::ForwardingRule - def all(region: nil, filter: nil, max_results: nil, - order_by: nil, page_token: nil) - + def all(region: nil, filter: nil, max_results: nil, order_by: nil, page_token: nil) opts = { :filter => filter, :max_results => max_results, @@ -15,7 +13,7 @@ def all(region: nil, filter: nil, max_results: nil, } if region - data = service.list_forwarding_rules(region, opts).items + data = service.list_forwarding_rules(region, opts).items || [] else data = [] service.list_aggregated_forwarding_rules(opts).items @@ -29,18 +27,16 @@ def all(region: nil, filter: nil, max_results: nil, end def get(identity, region = nil) - response = nil if region - response = service.get_forwarding_rule(identity, region).to_h + forwarding_rule = service.get_forwarding_rule(identity, region).to_h + return new(forwarding_rule) elsif identity response = all( :filter => "name eq #{identity}", :max_results => 1 - ).first - response = response.attributes unless response.nil? + ) + forwarding_rule = response.first unless response.empty? + return forwarding_rule end - - return nil if response.nil? - new(response) end end end diff --git a/lib/fog/compute/google/models/global_addresses.rb b/lib/fog/compute/google/models/global_addresses.rb index c04133c644..76ba049d7b 100644 --- a/lib/fog/compute/google/models/global_addresses.rb +++ b/lib/fog/compute/google/models/global_addresses.rb @@ -10,8 +10,9 @@ def all(options = {}) end def get(identity) - if address = service.get_global_address(identity).to_h - new(address) + if identity + address = service.get_global_address(identity).to_h + return new(address) end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 diff --git a/lib/fog/compute/google/models/global_forwarding_rules.rb b/lib/fog/compute/google/models/global_forwarding_rules.rb index c0e514079e..9009951fca 100644 --- a/lib/fog/compute/google/models/global_forwarding_rules.rb +++ b/lib/fog/compute/google/models/global_forwarding_rules.rb @@ -10,8 +10,9 @@ def all(opts = {}) end def get(identity) - if rule = service.get_global_forwarding_rule(identity).to_h - new(rule) + if identity + rule = service.get_global_forwarding_rule(identity).to_h + return new(rule) end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 diff --git a/lib/fog/compute/google/models/http_health_checks.rb b/lib/fog/compute/google/models/http_health_checks.rb index a981d2f3d0..eedc3f2102 100644 --- a/lib/fog/compute/google/models/http_health_checks.rb +++ b/lib/fog/compute/google/models/http_health_checks.rb @@ -10,9 +10,10 @@ def all(_filters = {}) end def get(identity) - response = service.get_http_health_check(identity) - return nil if response.nil? - new(response.to_h) + if identity + http_health_check = service.get_http_health_check(identity).to_h + return new(http_health_check) + end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 nil diff --git a/lib/fog/compute/google/models/images.rb b/lib/fog/compute/google/models/images.rb index 0da155d467..44473f210b 100644 --- a/lib/fog/compute/google/models/images.rb +++ b/lib/fog/compute/google/models/images.rb @@ -46,22 +46,30 @@ def current end def get(identity, project = nil) - project.nil? ? projects = all_projects : projects = [project] - data = nil - projects.each do |proj| - begin - data = service.get_image(identity, proj).to_h - data[:project] = proj - rescue ::Google::Apis::ClientError => e - next if e.status_code == 404 - break - else - break + if project + # TODO: Remove this - see #405 + image = service.get_image(identity, project).to_h + image[:project] = project + return new(image) + elsif identity + project.nil? ? projects = all_projects : projects = [project] + projects.each do |proj| + begin + response = service.get_image(identity, proj).to_h + # TODO: Remove this - see #405 + response[:project] = proj + image = response + return new(image) + rescue ::Google::Apis::ClientError => e + next if e.status_code == 404 + break + else + break + end end + # If nothing is found - return nil + nil end - - return nil if data.nil? - new(data) end def get_from_family(family, project = nil) diff --git a/lib/fog/compute/google/models/instance_group_managers.rb b/lib/fog/compute/google/models/instance_group_managers.rb index f96ac7896e..4102b8b2a7 100644 --- a/lib/fog/compute/google/models/instance_group_managers.rb +++ b/lib/fog/compute/google/models/instance_group_managers.rb @@ -26,11 +26,13 @@ def all(zone: nil, filter: nil, max_results: nil, def get(identity, zone = nil) if zone - if instance_group_manager = service.get_instance_group_manager(identity, zone) - new(instance_group_manager.to_h) - end - else - all(:filter => "name eq .*#{identity}").first + instance_group_manager = service.get_instance_group_manager(identity, zone).to_h + return new(instance_group_manager) + elsif identity + response = all(:filter => "name eq .*#{identity}", + :max_results => 1) + instance_group_manager = response.first unless response.empty? + return instance_group_manager end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 diff --git a/lib/fog/compute/google/models/instance_groups.rb b/lib/fog/compute/google/models/instance_groups.rb index 86d11ebd3b..c1d246ae45 100644 --- a/lib/fog/compute/google/models/instance_groups.rb +++ b/lib/fog/compute/google/models/instance_groups.rb @@ -18,16 +18,14 @@ def all(filters = {}) end def get(identity, zone = nil) - if zone.nil? - zones = service.list_aggregated_instance_groups(:filter => "name eq .*#{identity}").items - instance_groups = zones.each_value.map(&:instance_groups).compact.first - if instance_groups - zone = instance_groups.first.zone.split("/")[-1] - end - end - - if instance_group = service.get_instance_group(identity, zone) - new(instance_group.to_h) + if zone + instance_group = service.get_instance_group(identity, zone).to_h + new(instance_group) + elsif identity + response = all(:filter => "name eq #{identity}", + :max_results => 1) + instance_group = response.first unless response.empty? + return instance_group end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 diff --git a/lib/fog/compute/google/models/instance_templates.rb b/lib/fog/compute/google/models/instance_templates.rb index c5fc746dc3..d211c918e6 100644 --- a/lib/fog/compute/google/models/instance_templates.rb +++ b/lib/fog/compute/google/models/instance_templates.rb @@ -10,8 +10,9 @@ def all end def get(identity) - if instance_template = service.get_instance_template(identity) - new(instance_template.to_h) + if identity + instance_template = service.get_instance_template(identity).to_h + return new(instance_template) end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 diff --git a/lib/fog/compute/google/models/machine_types.rb b/lib/fog/compute/google/models/machine_types.rb index a03f4901c2..8d9e8490f1 100644 --- a/lib/fog/compute/google/models/machine_types.rb +++ b/lib/fog/compute/google/models/machine_types.rb @@ -23,10 +23,19 @@ def all(zone: nil, filter: nil, max_results: nil, order_by: nil, page_token: nil load(data.map(&:to_h) || []) end - def get(identity, zone) - machine_type = service.get_machine_type(identity, zone).to_h - return nil if machine_type.nil? - new(machine_type) + def get(identity, zone = nil) + if zone + machine_type = service.get_machine_type(identity, zone).to_h + return new(machine_type) + elsif identity + # This isn't very functional since it just shows the first available + # machine type globally, but needed due to overall compatibility + # See: https://github.com/fog/fog-google/issues/352 + response = all(:filter => "name eq #{identity}", + :max_results => 1) + machine_type = response.first unless response.empty? + return machine_type + end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 nil diff --git a/lib/fog/compute/google/models/networks.rb b/lib/fog/compute/google/models/networks.rb index c4956cbc3c..c8922f079e 100644 --- a/lib/fog/compute/google/models/networks.rb +++ b/lib/fog/compute/google/models/networks.rb @@ -10,8 +10,9 @@ def all end def get(identity) - if network = service.get_network(identity).to_h - new(network) + if identity + network = service.get_network(identity).to_h + return new(network) end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 diff --git a/lib/fog/compute/google/models/operations.rb b/lib/fog/compute/google/models/operations.rb index b864e2724b..ae3462c9bd 100644 --- a/lib/fog/compute/google/models/operations.rb +++ b/lib/fog/compute/google/models/operations.rb @@ -26,15 +26,15 @@ def all(zone: nil, region: nil, filter: nil, max_results: nil, def get(identity, zone = nil, region = nil) if !zone.nil? - response = service.get_zone_operation(zone, identity) + operation = service.get_zone_operation(zone, identity).to_h + return new(operation) elsif !region.nil? - response = service.get_region_operation(region, identity) - else - response = service.get_global_operation(identity) + operation = service.get_region_operation(region, identity).to_h + return new(operation) + elsif identity + operation = service.get_global_operation(identity).to_h + return new(operation) end - - return nil if response.nil? - new(response.to_h) rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 nil diff --git a/lib/fog/compute/google/models/projects.rb b/lib/fog/compute/google/models/projects.rb index a2e7ae9ba8..9de7110ad2 100644 --- a/lib/fog/compute/google/models/projects.rb +++ b/lib/fog/compute/google/models/projects.rb @@ -5,8 +5,9 @@ class Projects < Fog::Collection model Fog::Compute::Google::Project def get(identity) - if project = service.get_project(identity).to_h - new(project) + if identity + project = service.get_project(identity).to_h + return new(project) end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 diff --git a/lib/fog/compute/google/models/regions.rb b/lib/fog/compute/google/models/regions.rb index 3b062b48fe..1f8613e78f 100755 --- a/lib/fog/compute/google/models/regions.rb +++ b/lib/fog/compute/google/models/regions.rb @@ -10,8 +10,9 @@ def all end def get(identity) - if region = service.get_region(identity).to_h - new(region) + if identity + region = service.get_region(identity).to_h + return new(region) end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 diff --git a/lib/fog/compute/google/models/routes.rb b/lib/fog/compute/google/models/routes.rb index 751b27e4d5..2b6c35e4ea 100644 --- a/lib/fog/compute/google/models/routes.rb +++ b/lib/fog/compute/google/models/routes.rb @@ -10,8 +10,9 @@ def all end def get(identity) - if route = service.get_route(identity).to_h - new(route) + if identity + route = service.get_route(identity).to_h + return new(route) end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 diff --git a/lib/fog/compute/google/models/servers.rb b/lib/fog/compute/google/models/servers.rb index 53530748b3..81d5b7da35 100644 --- a/lib/fog/compute/google/models/servers.rb +++ b/lib/fog/compute/google/models/servers.rb @@ -28,15 +28,15 @@ def all(zone: nil, filter: nil, max_results: nil, # TODO: This method needs to take self_links as well as names def get(identity, zone = nil) - response = nil if zone - response = service.get_server(identity, zone).to_h - elseif identity - server = all(:filter => "name eq .*#{identity}").first - response = server.attributes if server + server = service.get_server(identity, zone).to_h + return new(server) + elsif identity + response = all(:filter => "name eq .*#{identity}", + :max_results => 1) + server = response.first unless response.empty? + return server end - return nil if response.nil? - new(response) rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 nil diff --git a/lib/fog/compute/google/models/snapshots.rb b/lib/fog/compute/google/models/snapshots.rb index 1b8023f30c..35bab9e809 100644 --- a/lib/fog/compute/google/models/snapshots.rb +++ b/lib/fog/compute/google/models/snapshots.rb @@ -17,10 +17,11 @@ def all load(items) end - def get(snap_id) - response = service.get_snapshot(snap_id) - return nil if response.nil? - new(response.to_h) + def get(identity) + if identity + snapshot = service.get_snapshot(identity).to_h + return new(snapshot) + end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 nil diff --git a/lib/fog/compute/google/models/ssl_certificates.rb b/lib/fog/compute/google/models/ssl_certificates.rb index 231cd5d328..97c51f056c 100644 --- a/lib/fog/compute/google/models/ssl_certificates.rb +++ b/lib/fog/compute/google/models/ssl_certificates.rb @@ -4,9 +4,10 @@ class Google class SslCertificates < Fog::Collection model Fog::Compute::Google::SslCertificate - def get(certificate_name) - if certificate = service.get_ssl_certificate(certificate_name) - new(certificate.to_h) + def get(identity) + if identity + ssl_certificate = service.get_ssl_certificate(identity).to_h + return new(ssl_certificate) end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 diff --git a/lib/fog/compute/google/models/subnetworks.rb b/lib/fog/compute/google/models/subnetworks.rb index c977e00491..5b46aab79b 100644 --- a/lib/fog/compute/google/models/subnetworks.rb +++ b/lib/fog/compute/google/models/subnetworks.rb @@ -23,9 +23,15 @@ def all(region: nil, filter: nil, max_results: nil, order_by: nil, page_token: n load(data || []) end - def get(identity, region) - if subnetwork = service.get_subnetwork(identity, region).to_h - new(subnetwork) + def get(identity, region = nil) + if region + subnetwork = service.get_subnetwork(identity, region).to_h + return new(subnetwork) + elsif identity + response = all(:filter => "name eq #{identity}", + :max_results => 1) + subnetwork = response.first unless response.empty? + return subnetwork end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 diff --git a/lib/fog/compute/google/models/target_http_proxies.rb b/lib/fog/compute/google/models/target_http_proxies.rb index 7af21c696e..e88f9e8813 100644 --- a/lib/fog/compute/google/models/target_http_proxies.rb +++ b/lib/fog/compute/google/models/target_http_proxies.rb @@ -12,7 +12,7 @@ def all(_filters = {}) def get(identity) if identity target_http_proxy = service.get_target_http_proxy(identity).to_h - new(target_http_proxy) if target_http_proxy + return new(target_http_proxy) end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 diff --git a/lib/fog/compute/google/models/target_https_proxies.rb b/lib/fog/compute/google/models/target_https_proxies.rb index f481d65ab3..3dda59d6e0 100644 --- a/lib/fog/compute/google/models/target_https_proxies.rb +++ b/lib/fog/compute/google/models/target_https_proxies.rb @@ -10,8 +10,9 @@ def all(_filters = {}) end def get(identity) - if target_https_proxy = service.get_target_https_proxy(identity).to_h - new(target_https_proxy) + if identity + target_https_proxy = service.get_target_https_proxy(identity).to_h + return new(target_https_proxy) end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 diff --git a/lib/fog/compute/google/models/target_instances.rb b/lib/fog/compute/google/models/target_instances.rb index 9a1d3c1e38..6713070f1e 100644 --- a/lib/fog/compute/google/models/target_instances.rb +++ b/lib/fog/compute/google/models/target_instances.rb @@ -26,16 +26,16 @@ def all(zone: nil, filter: nil, max_results: nil, order_by: nil, load(data) end - def get(target_instance, zone = nil) - response = nil + def get(identity, zone = nil) if zone - response = service.get_target_instance(target_instance, zone).to_h - else - response = all(:filter => "name eq #{target_instance}").first - response = response.attributes unless response.nil? + target_instance = service.get_target_instance(target_instance, zone).to_h + return new(target_instance) + elsif identity + response = all(:filter => "name eq #{identity}", + :max_results => 1) + target_instance = response.first unless response.empty? + return target_instance end - return nil if response.nil? - new(response) rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 nil diff --git a/lib/fog/compute/google/models/target_pools.rb b/lib/fog/compute/google/models/target_pools.rb index 533fb30627..4ee132ba3e 100644 --- a/lib/fog/compute/google/models/target_pools.rb +++ b/lib/fog/compute/google/models/target_pools.rb @@ -25,17 +25,14 @@ def all(region: nil, filter: nil, max_results: nil, order_by: nil, page_token: n end def get(identity, region = nil) - response = nil - if region.nil? - data = all(:filter => "name eq #{identity}").first - unless data.nil? - response = data.attributes - end - else - response = service.get_target_pool(identity, region).to_h + if region + target_pool = service.get_target_pool(identity, region).to_h + return new(target_pool) + elsif identity + response = all(:filter => "name eq #{identity}") + target_pool = response.first unless response.empty? + return target_pool end - return nil if response.nil? - new(response) rescue ::Google::Apis::ClientError => e raise e unless e.status_code = 404 nil diff --git a/lib/fog/compute/google/models/url_maps.rb b/lib/fog/compute/google/models/url_maps.rb index 3976883eb0..6f58fb6cb9 100644 --- a/lib/fog/compute/google/models/url_maps.rb +++ b/lib/fog/compute/google/models/url_maps.rb @@ -10,8 +10,9 @@ def all end def get(identity) - if url_map = service.get_url_map(identity).to_h - new(url_map) + if identity + url_map = service.get_url_map(identity).to_h + return new(url_map) end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 diff --git a/lib/fog/compute/google/models/zones.rb b/lib/fog/compute/google/models/zones.rb index dfa681334a..5c128af34f 100644 --- a/lib/fog/compute/google/models/zones.rb +++ b/lib/fog/compute/google/models/zones.rb @@ -10,8 +10,10 @@ def all end def get(identity) - data = service.get_zone(identity).to_h - new(data) + if identity + zone = service.get_zone(identity).to_h + new(zone) + end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 nil diff --git a/test/helpers/test_collection.rb b/test/helpers/test_collection.rb index f866a9fd20..8df68ea646 100644 --- a/test/helpers/test_collection.rb +++ b/test/helpers/test_collection.rb @@ -32,8 +32,6 @@ def test_enumerable end def test_nil_get - # Fixture for #33 - skip if @subject.method(:get).arity <= 1 assert_nil @subject.get(nil) elsif @subject.method(:get).arity == 2 diff --git a/test/unit/compute/test_common_collections.rb b/test/unit/compute/test_common_collections.rb index 6a20b4ffa4..38ebcdb49f 100644 --- a/test/unit/compute/test_common_collections.rb +++ b/test/unit/compute/test_common_collections.rb @@ -30,8 +30,6 @@ def test_common_methods end def test_collection_get_arguments - # TODO: Fixture for #352 - skip @collections.each do |klass| obj = klass.new assert_operator(obj.method(:get).arity, :<=, 1, From 3ad01e61271a37c30ff75850af07d858738d121c Mon Sep 17 00:00:00 2001 From: Artem Date: Sun, 22 Jul 2018 22:08:27 +1000 Subject: [PATCH 11/27] Fix instance group model all() and get() requests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit They didn’t match the common model methods and didn’t support parameters such as `max_results`, `order_by`, etc. This should be a no-op since a legacy options hash should be mapped to the new parameters in the same manner. --- .../compute/google/models/instance_groups.rb | 17 ++++++++++++----- .../requests/list_aggregated_instance_groups.rb | 3 +-- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/lib/fog/compute/google/models/instance_groups.rb b/lib/fog/compute/google/models/instance_groups.rb index c1d246ae45..8ea71f91b5 100644 --- a/lib/fog/compute/google/models/instance_groups.rb +++ b/lib/fog/compute/google/models/instance_groups.rb @@ -4,13 +4,20 @@ class Google class InstanceGroups < Fog::Collection model Fog::Compute::Google::InstanceGroup - def all(filters = {}) - if filters[:zone] - data = Array(service.list_instance_groups(filters[:zone])) + def all(zone: nil, filter: nil, max_results: nil, order_by: nil, page_token: nil) + opts = { + :filter => filter, + :max_results => max_results, + :order_by => order_by, + :page_token => page_token + } + + if zone + data = service.list_instance_groups(zone).items || [] else data = [] - service.list_aggregated_instance_groups.items.each_value do |group| - data.concat(group.instance_groups) if group.instance_groups + service.list_aggregated_instance_groups(opts).items.each_value do |group| + data.concat(group.instance_groups) if group && group.instance_groups end end diff --git a/lib/fog/compute/google/requests/list_aggregated_instance_groups.rb b/lib/fog/compute/google/requests/list_aggregated_instance_groups.rb index a0ad013ad7..bfee3f2f93 100644 --- a/lib/fog/compute/google/requests/list_aggregated_instance_groups.rb +++ b/lib/fog/compute/google/requests/list_aggregated_instance_groups.rb @@ -9,8 +9,7 @@ def list_aggregated_instance_groups(_options = {}) class Real def list_aggregated_instance_groups(options = {}) - @compute.list_aggregated_instance_groups(@project, - :filter => options[:filter]) + @compute.list_aggregated_instance_groups(@project, options) end end end From 1d59654260b1a49736e0e00e2de7711449d52129 Mon Sep 17 00:00:00 2001 From: Artem Date: Sun, 22 Jul 2018 22:12:00 +1000 Subject: [PATCH 12/27] Added additional collection tests To test both scoped and unscoped code paths See #352 for more context --- test/helpers/test_collection.rb | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/test/helpers/test_collection.rb b/test/helpers/test_collection.rb index 8df68ea646..0697b2ce78 100644 --- a/test/helpers/test_collection.rb +++ b/test/helpers/test_collection.rb @@ -13,8 +13,15 @@ def test_lifecycle assert_includes @subject.all.map(&:identity), one.identity assert_includes @subject.all.map(&:identity), two.identity - assert_equal one.identity, @factory.get(one.identity).identity - assert_equal two.identity, @factory.get(two.identity).identity + assert_equal one.identity, @subject.get(one.identity).identity + assert_equal two.identity, @subject.get(two.identity).identity + + # Some factories that have scoped parameters (zone, region) have a special + # `get` method defined in the factory to pass the correct parameters in + if @factory.respond_to?(:get) + assert_equal one.identity, @factory.get(one.identity).identity + assert_equal two.identity, @factory.get(two.identity).identity + end one.destroy two.destroy From 1527a906fa218fa82b4d29590ae1fca7e3414b35 Mon Sep 17 00:00:00 2001 From: Artem Date: Thu, 26 Jul 2018 20:33:37 +1000 Subject: [PATCH 13/27] Update Travis config Issue was fixed it seems --- .travis.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index 3ede3c1fa0..da225d3415 100644 --- a/.travis.yml +++ b/.travis.yml @@ -12,9 +12,6 @@ matrix: - rvm: 2.5 - rvm: jruby-head allow_failures: - # allow 2.5 to fail until Travis CI is fixed. - # See travis-ci/travis-ci#8969 - - rvm: 2.5 - rvm: jruby-head notifications: email: false From 3d3e6448ced10e2626dcf523d435660ab2bb45eb Mon Sep 17 00:00:00 2001 From: Artem Date: Thu, 26 Jul 2018 20:36:41 +1000 Subject: [PATCH 14/27] Fix unit test client init Forgot to set `google_project` since we usually have a ~/.fog file --- test/unit/compute/test_common_collections.rb | 3 ++- test/unit/compute/test_common_models.rb | 3 ++- test/unit/compute/test_server.rb | 3 ++- test/unit/dns/test_common_collections.rb | 3 ++- test/unit/monitoring/test_comon_collections.rb | 3 ++- test/unit/pubsub/test_common_collections.rb | 2 +- test/unit/sql/test_common_collections.rb | 2 +- test/unit/storage/test_common_json_collections.rb | 3 ++- 8 files changed, 14 insertions(+), 8 deletions(-) diff --git a/test/unit/compute/test_common_collections.rb b/test/unit/compute/test_common_collections.rb index 38ebcdb49f..707ce93021 100644 --- a/test/unit/compute/test_common_collections.rb +++ b/test/unit/compute/test_common_collections.rb @@ -5,7 +5,8 @@ class UnitTestCollections < MiniTest::Test def setup Fog.mock! - @client = Fog::Compute.new(:provider => "Google", :google_project => "foo") + @client = Fog::Compute.new(provider: "google", + google_project: "foo") # Projects do not have a "list" method in compute API exceptions = [Fog::Compute::Google::Projects] diff --git a/test/unit/compute/test_common_models.rb b/test/unit/compute/test_common_models.rb index a3b2cbf907..93b578778c 100644 --- a/test/unit/compute/test_common_models.rb +++ b/test/unit/compute/test_common_models.rb @@ -3,7 +3,8 @@ class UnitTestModels < MiniTest::Test def setup Fog.mock! - @client = Fog::Compute.new(:provider => "Google", :google_project => "foo") + @client = Fog::Compute.new(provider: "google", + google_project: "foo") # Do not test models that do not have a create method in API exceptions = [Fog::Compute::Google::MachineType, diff --git a/test/unit/compute/test_server.rb b/test/unit/compute/test_server.rb index 4f6c02531a..33dc366818 100644 --- a/test/unit/compute/test_server.rb +++ b/test/unit/compute/test_server.rb @@ -3,7 +3,8 @@ class UnitTestServer < MiniTest::Test def setup Fog.mock! - @client = Fog::Compute.new(:provider => "Google", :google_project => "foo") + @client = Fog::Compute.new(provider: "google", + google_project: "foo") end def teardown diff --git a/test/unit/dns/test_common_collections.rb b/test/unit/dns/test_common_collections.rb index d0f2a15f35..98e457563f 100644 --- a/test/unit/dns/test_common_collections.rb +++ b/test/unit/dns/test_common_collections.rb @@ -4,7 +4,8 @@ class UnitTestDNSCollections < MiniTest::Test def setup Fog.mock! - @client = Fog::DNS.new(provider: "google") + @client = Fog::DNS.new(provider: "google", + google_project: "foo") # DNS Projects API does not support 'list', so 'all' method is not possible exceptions = [Fog::DNS::Google::Projects] diff --git a/test/unit/monitoring/test_comon_collections.rb b/test/unit/monitoring/test_comon_collections.rb index 0ce3f5b754..68b41c5b6b 100644 --- a/test/unit/monitoring/test_comon_collections.rb +++ b/test/unit/monitoring/test_comon_collections.rb @@ -4,7 +4,8 @@ class UnitTestMonitoringCollections < MiniTest::Test def setup Fog.mock! - @client = Fog::Monitoring.new(provider: "google") + @client = Fog::Monitoring.new(provider: "google", + google_project: "foo") # TimeSeries API has no 'get' method exceptions = [Fog::Google::Monitoring::TimeseriesCollection] diff --git a/test/unit/pubsub/test_common_collections.rb b/test/unit/pubsub/test_common_collections.rb index aea39e2d28..a00673c009 100644 --- a/test/unit/pubsub/test_common_collections.rb +++ b/test/unit/pubsub/test_common_collections.rb @@ -4,7 +4,7 @@ class UnitTestPubsubCollections < MiniTest::Test def setup Fog.mock! - @client = Fog::Google::Pubsub.new + @client = Fog::Google::Pubsub.new(google_project: "foo") exceptions = [] # Enumerate all descendants of Fog::Collection diff --git a/test/unit/sql/test_common_collections.rb b/test/unit/sql/test_common_collections.rb index b3429d97d6..677aadddad 100644 --- a/test/unit/sql/test_common_collections.rb +++ b/test/unit/sql/test_common_collections.rb @@ -4,7 +4,7 @@ class UnitTestSQLCollections < MiniTest::Test def setup Fog.mock! - @client = Fog::Google::SQL.new + @client = Fog::Google::SQL.new(google_project: "foo") # SQL Users API doesn't have a get method # SQL Flags API has only a 'list' method diff --git a/test/unit/storage/test_common_json_collections.rb b/test/unit/storage/test_common_json_collections.rb index 2a71d498a7..d1cc34aed9 100644 --- a/test/unit/storage/test_common_json_collections.rb +++ b/test/unit/storage/test_common_json_collections.rb @@ -4,7 +4,8 @@ class UnitTestStorageJSONCollections < MiniTest::Test def setup Fog.mock! - @client = Fog::Storage.new(provider: "google") + @client = Fog::Storage.new(provider: "google", + google_project: "foo") # Enumerate all descendants of Fog::Collection descendants = ObjectSpace.each_object(Fog::Collection.singleton_class) From 1916f01f927a9e56bd66992a648e420609cc040f Mon Sep 17 00:00:00 2001 From: Artem Yakimenko Date: Fri, 3 Aug 2018 15:28:16 +1000 Subject: [PATCH 15/27] =?UTF-8?q?Remove=20temp=20fixtures=20from=20most=20?= =?UTF-8?q?API=E2=80=99s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SQL is still a problem due to API missing AggregatedList --- test/helpers/test_collection.rb | 8 +------- test/unit/monitoring/test_comon_collections.rb | 2 -- test/unit/pubsub/test_common_collections.rb | 1 - 3 files changed, 1 insertion(+), 10 deletions(-) diff --git a/test/helpers/test_collection.rb b/test/helpers/test_collection.rb index 0697b2ce78..af008d413d 100644 --- a/test/helpers/test_collection.rb +++ b/test/helpers/test_collection.rb @@ -39,13 +39,7 @@ def test_enumerable end def test_nil_get - if @subject.method(:get).arity <= 1 - assert_nil @subject.get(nil) - elsif @subject.method(:get).arity == 2 - assert_nil @subject.get(nil, nil) - else - fail "Unexpected number of required get parameters" - end + assert_nil @subject.get(nil) end def teardown diff --git a/test/unit/monitoring/test_comon_collections.rb b/test/unit/monitoring/test_comon_collections.rb index 68b41c5b6b..2ab29d27a4 100644 --- a/test/unit/monitoring/test_comon_collections.rb +++ b/test/unit/monitoring/test_comon_collections.rb @@ -30,8 +30,6 @@ def test_common_methods end def test_collection_get_arguments - # TODO: Fixture for #352 - skip @collections.each do |klass| obj = klass.new assert_operator(obj.method(:get).arity, :<=, 1, diff --git a/test/unit/pubsub/test_common_collections.rb b/test/unit/pubsub/test_common_collections.rb index a00673c009..9a32ef06a4 100644 --- a/test/unit/pubsub/test_common_collections.rb +++ b/test/unit/pubsub/test_common_collections.rb @@ -28,7 +28,6 @@ def test_common_methods end def test_collection_get_arguments - # TODO: Fixture for #352 @collections.each do |klass| obj = klass.new assert_operator(obj.method(:get).arity, :<=, 1, From 510f9eb4c52f7989a3ce9a4bc83265ddf2dfc2c8 Mon Sep 17 00:00:00 2001 From: Artem Yakimenko Date: Fri, 3 Aug 2018 16:27:39 +1000 Subject: [PATCH 16/27] Refactor unit tests to not entirely skip exceptions --- test/unit/compute/test_common_collections.rb | 13 +++++++----- test/unit/dns/test_common_collections.rb | 11 ++++++---- .../unit/monitoring/test_comon_collections.rb | 18 +++++++++++------ test/unit/pubsub/test_common_collections.rb | 5 ++--- test/unit/sql/test_common_collections.rb | 20 +++++++++++-------- 5 files changed, 41 insertions(+), 26 deletions(-) diff --git a/test/unit/compute/test_common_collections.rb b/test/unit/compute/test_common_collections.rb index 707ce93021..68dda57bec 100644 --- a/test/unit/compute/test_common_collections.rb +++ b/test/unit/compute/test_common_collections.rb @@ -8,21 +8,24 @@ def setup @client = Fog::Compute.new(provider: "google", google_project: "foo") - # Projects do not have a "list" method in compute API - exceptions = [Fog::Compute::Google::Projects] + # Exceptions that do not pass test_common_methods: + # + # Projects do not have a "list" method in compute API, so 'all' is not implemented + @common_method_exceptions = [Fog::Compute::Google::Projects] # Enumerate all descendants of Fog::Collection descendants = ObjectSpace.each_object(Fog::Collection.singleton_class).to_a - @collections = descendants.select { |d| d.name.match /Fog::Compute::Google/ } - exceptions + @collections = descendants.select { |d| d.name.match /Fog::Compute::Google/ } end def teardown Fog.unmock! end + # This tests whether Fog::Compute::Google collections have common lifecycle methods def test_common_methods - # This tests whether Fog::Compute::Google collections have common lifecycle methods - @collections.each do |klass| + subjects = @collections - @common_method_exceptions + subjects.each do |klass| obj = klass.new assert obj.respond_to?(:all), "#{klass} should have an .all method" assert obj.respond_to?(:get), "#{klass} should have a .get method" diff --git a/test/unit/dns/test_common_collections.rb b/test/unit/dns/test_common_collections.rb index 98e457563f..e84158c1e9 100644 --- a/test/unit/dns/test_common_collections.rb +++ b/test/unit/dns/test_common_collections.rb @@ -7,21 +7,24 @@ def setup @client = Fog::DNS.new(provider: "google", google_project: "foo") + # Exceptions that do not pass test_common_methods: + # # DNS Projects API does not support 'list', so 'all' method is not possible - exceptions = [Fog::DNS::Google::Projects] + @common_methods_exceptions = [Fog::DNS::Google::Projects] # Enumerate all descendants of Fog::Collection descendants = ObjectSpace.each_object(Fog::Collection.singleton_class) - @collections = descendants.select { |d| d.name.match /Fog::DNS::Google/ } - exceptions + @collections = descendants.select { |d| d.name.match /Fog::DNS::Google/ } end def teardown Fog.unmock! end + # This tests whether Fog::Compute::Google collections have common lifecycle methods def test_common_methods - # This tests whether Fog::Compute::Google collections have common lifecycle methods - @collections.each do |klass| + subjects = @collections - @common_methods_exceptions + subjects.each do |klass| obj = klass.new assert obj.respond_to?(:all), "#{klass} should have an .all method" assert obj.respond_to?(:get), "#{klass} should have a .get method" diff --git a/test/unit/monitoring/test_comon_collections.rb b/test/unit/monitoring/test_comon_collections.rb index 2ab29d27a4..dabb1f8d14 100644 --- a/test/unit/monitoring/test_comon_collections.rb +++ b/test/unit/monitoring/test_comon_collections.rb @@ -7,21 +7,25 @@ def setup @client = Fog::Monitoring.new(provider: "google", google_project: "foo") + # Exceptions that do not pass test_common_methods: + # # TimeSeries API has no 'get' method - exceptions = [Fog::Google::Monitoring::TimeseriesCollection] + @common_method_exceptions = [Fog::Google::Monitoring::TimeseriesCollection] # Enumerate all descendants of Fog::Collection descendants = ObjectSpace.each_object(Fog::Collection.singleton_class).to_a - @collections = descendants.select { |d| d.name.match /Fog::Google::Monitoring/ } - exceptions + @collections = descendants.select { |d| d.name.match /Fog::Google::Monitoring/ } end def teardown Fog.unmock! end + # This tests whether Fog::Compute::Google collections have common lifecycle methods def test_common_methods - # This tests whether Fog::Compute::Google collections have common lifecycle methods - @collections.each do |klass| + subjects = @collections - @common_method_exceptions + + subjects.each do |klass| obj = klass.new assert obj.respond_to?(:all), "#{klass} should have an .all method" assert obj.respond_to?(:get), "#{klass} should have a .get method" @@ -32,8 +36,10 @@ def test_common_methods def test_collection_get_arguments @collections.each do |klass| obj = klass.new - assert_operator(obj.method(:get).arity, :<=, 1, - "#{klass} should have at most 1 required parameter in get()") + if obj.respond_to?(:get) + assert_operator(obj.method(:get).arity, :<=, 1, + "#{klass} should have at most 1 required parameter in get()") + end end end end diff --git a/test/unit/pubsub/test_common_collections.rb b/test/unit/pubsub/test_common_collections.rb index 9a32ef06a4..fbc27abf35 100644 --- a/test/unit/pubsub/test_common_collections.rb +++ b/test/unit/pubsub/test_common_collections.rb @@ -5,12 +5,11 @@ class UnitTestPubsubCollections < MiniTest::Test def setup Fog.mock! @client = Fog::Google::Pubsub.new(google_project: "foo") - - exceptions = [] + # Enumerate all descendants of Fog::Collection descendants = ObjectSpace.each_object(Fog::Collection.singleton_class) - @collections = descendants.select { |d| d.name.match /Fog::Google::Pubsub/ } - exceptions + @collections = descendants.select { |d| d.name.match /Fog::Google::Pubsub/ } end def teardown diff --git a/test/unit/sql/test_common_collections.rb b/test/unit/sql/test_common_collections.rb index 677aadddad..00b9a7905a 100644 --- a/test/unit/sql/test_common_collections.rb +++ b/test/unit/sql/test_common_collections.rb @@ -6,24 +6,28 @@ def setup Fog.mock! @client = Fog::Google::SQL.new(google_project: "foo") - # SQL Users API doesn't have a get method - # SQL Flags API has only a 'list' method - exceptions = [Fog::Google::SQL::Users, - Fog::Google::SQL::Tiers, - Fog::Google::SQL::Flags] + # Exceptions that do not pass test_common_methods: + # + # SQL Users API doesn't have a 'get' method + # SQL Flags API has only one method - 'list' + # SQL Tiers API has only one method - 'list' + @common_method_exceptions = [Fog::Google::SQL::Users, + Fog::Google::SQL::Tiers, + Fog::Google::SQL::Flags] # Enumerate all descendants of Fog::Collection descendants = ObjectSpace.each_object(Fog::Collection.singleton_class) - @collections = descendants.select { |d| d.name.match /Fog::Google::SQL/ } - exceptions + @collections = descendants.select { |d| d.name.match /Fog::Google::SQL/ } end def teardown Fog.unmock! end + # This tests whether Fog::Compute::Google collections have common lifecycle methods def test_common_methods - # This tests whether Fog::Compute::Google collections have common lifecycle methods - @collections.each do |klass| + subjects = @collections - @common_method_exceptions + subjects.each do |klass| obj = klass.new assert obj.respond_to?(:all), "#{klass} should have an .all method" assert obj.respond_to?(:get), "#{klass} should have a .get method" From db50b8be2458a352f53d48441eda986dff81b7c0 Mon Sep 17 00:00:00 2001 From: Artem Yakimenko Date: Mon, 17 Sep 2018 14:57:28 +1000 Subject: [PATCH 17/27] Disable Style/RedundantReturn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Return reads better in fetch logic, so I think I’ll disable this. We can always re-enable if it becomes a problem. --- .rubocop.yml | 3 +++ lib/fog/compute/google/models/instance_groups.rb | 9 +++++---- test/integration/factories/collection_factory.rb | 2 +- test/unit/compute/test_common_collections.rb | 2 +- test/unit/compute/test_common_models.rb | 2 +- test/unit/dns/test_common_collections.rb | 2 +- test/unit/monitoring/test_comon_collections.rb | 2 +- test/unit/pubsub/test_common_collections.rb | 4 ++-- test/unit/sql/test_common_collections.rb | 2 +- test/unit/storage/test_common_json_collections.rb | 2 +- test/unit/storage/test_common_xml_collections.rb | 2 +- 11 files changed, 18 insertions(+), 14 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index a4145c0568..7f796a7e73 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -6,6 +6,9 @@ Style/ConditionalAssignment: SingleLineConditionsOnly: true EnforcedStyle: assign_inside_condition +Style/RedundantReturn: + Enabled: false + Style/FormatString: Enabled: false diff --git a/lib/fog/compute/google/models/instance_groups.rb b/lib/fog/compute/google/models/instance_groups.rb index 8ea71f91b5..ba0e7f1e10 100644 --- a/lib/fog/compute/google/models/instance_groups.rb +++ b/lib/fog/compute/google/models/instance_groups.rb @@ -6,10 +6,10 @@ class InstanceGroups < Fog::Collection def all(zone: nil, filter: nil, max_results: nil, order_by: nil, page_token: nil) opts = { - :filter => filter, - :max_results => max_results, - :order_by => order_by, - :page_token => page_token + :filter => filter, + :max_results => max_results, + :order_by => order_by, + :page_token => page_token } if zone @@ -36,6 +36,7 @@ def get(identity, zone = nil) end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 + nil end diff --git a/test/integration/factories/collection_factory.rb b/test/integration/factories/collection_factory.rb index bc436216f8..651fdf4ee5 100644 --- a/test/integration/factories/collection_factory.rb +++ b/test/integration/factories/collection_factory.rb @@ -12,7 +12,7 @@ def initialize(subject, example) # @param async [FalseClass or TrueClass] perform resource destruction asynchronously def cleanup(async = false) suit_name = @example.gsub(/\W/, "").tr("_", "-").downcase.split('-')[0] - resources = @subject.all.select { |resource| resource.name.match /#{PREFIX}-[0-9]*-#{suit_name}/ } + resources = @subject.all.select { |resource| resource.name.match(/#{PREFIX}-[0-9]*-#{suit_name}/) } if DEBUG p "Cleanup invoked in #{self} for example: #{@example}" p "Resources to be deleted: #{resources.map { |r| r.name }}" diff --git a/test/unit/compute/test_common_collections.rb b/test/unit/compute/test_common_collections.rb index 68dda57bec..0e18e49ba5 100644 --- a/test/unit/compute/test_common_collections.rb +++ b/test/unit/compute/test_common_collections.rb @@ -15,7 +15,7 @@ def setup # Enumerate all descendants of Fog::Collection descendants = ObjectSpace.each_object(Fog::Collection.singleton_class).to_a - @collections = descendants.select { |d| d.name.match /Fog::Compute::Google/ } + @collections = descendants.select { |d| d.name.match(/Fog::Compute::Google/) } end def teardown diff --git a/test/unit/compute/test_common_models.rb b/test/unit/compute/test_common_models.rb index 93b578778c..cce8ab5b7c 100644 --- a/test/unit/compute/test_common_models.rb +++ b/test/unit/compute/test_common_models.rb @@ -17,7 +17,7 @@ def setup # Enumerate all descendants of Fog::Model descendants = ObjectSpace.each_object(Fog::Model.singleton_class).to_a - @models = descendants.select { |d| d.name.match /Fog::Compute::Google/ } - exceptions + @models = descendants.select { |d| d.name.match(/Fog::Compute::Google/) } - exceptions end def teardown diff --git a/test/unit/dns/test_common_collections.rb b/test/unit/dns/test_common_collections.rb index e84158c1e9..dd8ff992c2 100644 --- a/test/unit/dns/test_common_collections.rb +++ b/test/unit/dns/test_common_collections.rb @@ -14,7 +14,7 @@ def setup # Enumerate all descendants of Fog::Collection descendants = ObjectSpace.each_object(Fog::Collection.singleton_class) - @collections = descendants.select { |d| d.name.match /Fog::DNS::Google/ } + @collections = descendants.select { |d| d.name.match(/Fog::DNS::Google/) } end def teardown diff --git a/test/unit/monitoring/test_comon_collections.rb b/test/unit/monitoring/test_comon_collections.rb index dabb1f8d14..9ee054b306 100644 --- a/test/unit/monitoring/test_comon_collections.rb +++ b/test/unit/monitoring/test_comon_collections.rb @@ -14,7 +14,7 @@ def setup # Enumerate all descendants of Fog::Collection descendants = ObjectSpace.each_object(Fog::Collection.singleton_class).to_a - @collections = descendants.select { |d| d.name.match /Fog::Google::Monitoring/ } + @collections = descendants.select { |d| d.name.match(/Fog::Google::Monitoring/) } end def teardown diff --git a/test/unit/pubsub/test_common_collections.rb b/test/unit/pubsub/test_common_collections.rb index fbc27abf35..828f853f18 100644 --- a/test/unit/pubsub/test_common_collections.rb +++ b/test/unit/pubsub/test_common_collections.rb @@ -5,11 +5,11 @@ class UnitTestPubsubCollections < MiniTest::Test def setup Fog.mock! @client = Fog::Google::Pubsub.new(google_project: "foo") - + # Enumerate all descendants of Fog::Collection descendants = ObjectSpace.each_object(Fog::Collection.singleton_class) - @collections = descendants.select { |d| d.name.match /Fog::Google::Pubsub/ } + @collections = descendants.select { |d| d.name.match(/Fog::Google::Pubsub/) } end def teardown diff --git a/test/unit/sql/test_common_collections.rb b/test/unit/sql/test_common_collections.rb index 00b9a7905a..997ad17ee6 100644 --- a/test/unit/sql/test_common_collections.rb +++ b/test/unit/sql/test_common_collections.rb @@ -17,7 +17,7 @@ def setup # Enumerate all descendants of Fog::Collection descendants = ObjectSpace.each_object(Fog::Collection.singleton_class) - @collections = descendants.select { |d| d.name.match /Fog::Google::SQL/ } + @collections = descendants.select { |d| d.name.match(/Fog::Google::SQL/) } end def teardown diff --git a/test/unit/storage/test_common_json_collections.rb b/test/unit/storage/test_common_json_collections.rb index d1cc34aed9..8c3a6b7a34 100644 --- a/test/unit/storage/test_common_json_collections.rb +++ b/test/unit/storage/test_common_json_collections.rb @@ -11,7 +11,7 @@ def setup descendants = ObjectSpace.each_object(Fog::Collection.singleton_class) @collections = descendants.select do |d| - d.name.match /Fog::Storage::GoogleJSON/ + d.name.match(/Fog::Storage::GoogleJSON/) end end diff --git a/test/unit/storage/test_common_xml_collections.rb b/test/unit/storage/test_common_xml_collections.rb index 95c75f0eee..e934d08a77 100644 --- a/test/unit/storage/test_common_xml_collections.rb +++ b/test/unit/storage/test_common_xml_collections.rb @@ -12,7 +12,7 @@ def setup descendants = ObjectSpace.each_object(Fog::Collection.singleton_class) @collections = descendants.select do |d| - d.name.match /Fog::Storage::GoogleXML/ + d.name.match(/Fog::Storage::GoogleXML/) end end From 6b1ea84d9650c346a94c615ba9761df872855465 Mon Sep 17 00:00:00 2001 From: Artem Yakimenko Date: Tue, 18 Sep 2018 14:27:44 +1000 Subject: [PATCH 18/27] Bring the IGM query to common format MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It’s less elegant but should more gracefully fail if service.list_instance_group_managers(zone, opts).items returns nonsense (`false` for example) --- lib/fog/compute/google/models/instance_group_managers.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/fog/compute/google/models/instance_group_managers.rb b/lib/fog/compute/google/models/instance_group_managers.rb index 4102b8b2a7..169a71e86c 100644 --- a/lib/fog/compute/google/models/instance_group_managers.rb +++ b/lib/fog/compute/google/models/instance_group_managers.rb @@ -12,10 +12,11 @@ def all(zone: nil, filter: nil, max_results: nil, :order_by => order_by, :page_token => page_token } - data = [] + if zone - data += service.list_instance_group_managers(zone, opts).items || [] + data = service.list_instance_group_managers(zone, opts).items || [] else + data = [] service.list_aggregated_instance_group_managers(opts).items.each_value do |group| data.concat(group.instance_group_managers) if group.instance_group_managers end From 6967fddc58feab8e61b3bdb709c1147cce5c491b Mon Sep 17 00:00:00 2001 From: Artem Yakimenko Date: Tue, 18 Sep 2018 14:30:43 +1000 Subject: [PATCH 19/27] Fix a typo in IGM test class --- .../compute/instance_groups/test_instance_group_managers.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/compute/instance_groups/test_instance_group_managers.rb b/test/integration/compute/instance_groups/test_instance_group_managers.rb index aea91ef088..13d811eb5c 100644 --- a/test/integration/compute/instance_groups/test_instance_group_managers.rb +++ b/test/integration/compute/instance_groups/test_instance_group_managers.rb @@ -1,7 +1,7 @@ require "helpers/integration_test_helper" require "integration/factories/instance_group_manager_factory" -class TestInstanceGroupManager < FogIntegrationTest +class TestInstanceGroupManagers < FogIntegrationTest include TestCollection def setup From 3d89ff7a8d8486af81e433c98108e0d171433380 Mon Sep 17 00:00:00 2001 From: Artem Yakimenko Date: Tue, 18 Sep 2018 14:40:03 +1000 Subject: [PATCH 20/27] Fix typo in operations.all when using regional selector --- lib/fog/compute/google/models/operations.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/fog/compute/google/models/operations.rb b/lib/fog/compute/google/models/operations.rb index ae3462c9bd..00cadf6e71 100644 --- a/lib/fog/compute/google/models/operations.rb +++ b/lib/fog/compute/google/models/operations.rb @@ -16,7 +16,7 @@ def all(zone: nil, region: nil, filter: nil, max_results: nil, if zone data = service.list_zone_operations(zone, opts).to_h[:items] elsif region - data = service.list_region_operations(regions, opts).to_h[:items] + data = service.list_region_operations(region, opts).to_h[:items] else data = service.list_global_operations(opts).to_h[:items] end From f1fcf4592559f055e95b1eb5227d8efafa6b95a5 Mon Sep 17 00:00:00 2001 From: Artem Yakimenko Date: Tue, 18 Sep 2018 14:40:16 +1000 Subject: [PATCH 21/27] Fix typo in Operations test class --- test/integration/compute/core_compute/test_operations.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/compute/core_compute/test_operations.rb b/test/integration/compute/core_compute/test_operations.rb index ec1ade4695..217bdefe66 100644 --- a/test/integration/compute/core_compute/test_operations.rb +++ b/test/integration/compute/core_compute/test_operations.rb @@ -1,6 +1,6 @@ require "helpers/integration_test_helper" -class TestRoutes < FogIntegrationTest +class TestOperations < FogIntegrationTest def setup @subject = Fog::Compute[:google].operations end From 162cb0fa926b86c1c80529c6d4715d53864aa0d7 Mon Sep 17 00:00:00 2001 From: Artem Yakimenko Date: Thu, 20 Sep 2018 13:38:26 +1000 Subject: [PATCH 22/27] Reverse scoping logic in subnetworks To work in the same way as all other collections, e.g. `Servers` --- lib/fog/compute/google/models/subnetworks.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/fog/compute/google/models/subnetworks.rb b/lib/fog/compute/google/models/subnetworks.rb index 5b46aab79b..d0911631be 100644 --- a/lib/fog/compute/google/models/subnetworks.rb +++ b/lib/fog/compute/google/models/subnetworks.rb @@ -12,15 +12,15 @@ def all(region: nil, filter: nil, max_results: nil, order_by: nil, page_token: n :page_token => page_token } - if region.nil? + if region + data = service.list_subnetworks(region, filters).to_h[:items] || [] + else data = [] service.list_aggregated_subnetworks(filters).to_h[:items].each_value do |region_obj| data.concat(region_obj[:subnetworks]) if region_obj[:subnetworks] end - else - data = service.list_subnetworks(region, filters).to_h[:items] end - load(data || []) + load(data) end def get(identity, region = nil) From d9fafff7fe1349e7bf1b3b747b4fa4f450697f1c Mon Sep 17 00:00:00 2001 From: Artem Yakimenko Date: Thu, 20 Sep 2018 13:41:52 +1000 Subject: [PATCH 23/27] Add additional coverage for scoped requests We should also test that scoped get() and all() requests also return valid results --- test/helpers/test_collection.rb | 11 +++++++++++ .../compute/core_compute/test_disk_types.rb | 9 +++++++++ .../compute/core_compute/test_machine_types.rb | 13 +++++++++++++ .../compute/core_compute/test_operations.rb | 18 ++++++++++++++++++ .../integration/factories/addresses_factory.rb | 4 ++++ test/integration/factories/disks_factory.rb | 4 ++++ .../factories/forwarding_rules_factory.rb | 4 ++++ test/integration/factories/images_factory.rb | 4 ++++ .../instance_group_manager_factory.rb | 8 ++++++++ .../factories/instance_groups_factory.rb | 4 ++++ test/integration/factories/servers_factory.rb | 8 ++++++++ .../factories/subnetworks_factory.rb | 4 ++++ .../factories/target_instances_factory.rb | 4 ++++ .../factories/target_pools_factory.rb | 4 ++++ 14 files changed, 99 insertions(+) diff --git a/test/helpers/test_collection.rb b/test/helpers/test_collection.rb index af008d413d..e892221622 100644 --- a/test/helpers/test_collection.rb +++ b/test/helpers/test_collection.rb @@ -23,6 +23,17 @@ def test_lifecycle assert_equal two.identity, @factory.get(two.identity).identity end + # Some factories that have scoped parameters (zone, region) have a special + # `all` method defined in the factory to pass the correct parameters in + if @factory.respond_to?(:all) + subject_list = @subject.all + scoped_subject_list = @factory.all + + # Assert that whatever .all(scope) returns is a subset of .all + assert(scoped_subject_list.all? { |x| subject_list.include? x }, + "Output of @factory.all must be a subset of @subject.all") + end + one.destroy two.destroy diff --git a/test/integration/compute/core_compute/test_disk_types.rb b/test/integration/compute/core_compute/test_disk_types.rb index ad30fd643e..fd0ff4b883 100644 --- a/test/integration/compute/core_compute/test_disk_types.rb +++ b/test/integration/compute/core_compute/test_disk_types.rb @@ -16,6 +16,15 @@ def test_all "Number of all disk type references should be greater or equal to test zones * disk types") end + def test_scoped_all + subject_list = @subject.all + scoped_subject_list = @subject.all(zone: TEST_ZONE) + + # Assert that whatever .all(scope) returns is a subset of .all + assert(scoped_subject_list.all? { |x| subject_list.include? x }, + "Output of @subject.all(zone:#{TEST_ZONE}) must be a subset of @subject.all") + end + def test_get NAMES.each do |name| ZONES.each do |zone| diff --git a/test/integration/compute/core_compute/test_machine_types.rb b/test/integration/compute/core_compute/test_machine_types.rb index 591fd94136..1e46460608 100644 --- a/test/integration/compute/core_compute/test_machine_types.rb +++ b/test/integration/compute/core_compute/test_machine_types.rb @@ -23,6 +23,15 @@ def test_all "Number of all machine types should be greater or equal to test zones * machine_types") end + def test_scoped_all + subject_list = @subject.all + scoped_subject_list = @subject.all(zone: TEST_ZONE) + + # Assert that whatever .all(scope) returns is a subset of .all + assert(scoped_subject_list.all? { |x| subject_list.include? x }, + "Output of @subject.all(zone:#{TEST_ZONE}) must be a subset of @subject.all") + end + def test_get # This tests only in last zone since not all zones contain all machine types NAMES.each do |name| @@ -39,4 +48,8 @@ def test_bad_get def test_enumerable assert_respond_to @subject, :each end + + def test_nil_get + assert_nil @subject.get(nil) + end end diff --git a/test/integration/compute/core_compute/test_operations.rb b/test/integration/compute/core_compute/test_operations.rb index 217bdefe66..286823930b 100644 --- a/test/integration/compute/core_compute/test_operations.rb +++ b/test/integration/compute/core_compute/test_operations.rb @@ -17,6 +17,24 @@ def test_get end end + def test_zone_scoped_all + subject_list = @subject.all + scoped_subject_list = @subject.all(zone: TEST_ZONE) + + # Assert that whatever .all(scope) returns is a subset of .all + assert(scoped_subject_list.all? { |x| subject_list.include? x }, + "Output of @subject.all(zone:#{TEST_ZONE}) must be a subset of @subject.all") + end + + def test_region_scoped_all + subject_list = @subject.all + scoped_subject_list = @subject.all(region: TEST_REGION) + + # Assert that whatever .all(scope) returns is a subset of .all + assert(scoped_subject_list.all? { |x| subject_list.include? x }, + "Output of @subject.all(region:#{TEST_REGION}) must be a subset of @subject.all") + end + def test_bad_get assert_nil @subject.get("bad-name") end diff --git a/test/integration/factories/addresses_factory.rb b/test/integration/factories/addresses_factory.rb index bd2ffc175c..bc99115119 100644 --- a/test/integration/factories/addresses_factory.rb +++ b/test/integration/factories/addresses_factory.rb @@ -9,6 +9,10 @@ def get(identity) @subject.get(identity, TEST_REGION) end + def all + @subject.all(region: TEST_REGION) + end + def params { :name => resource_name, :region => TEST_REGION } diff --git a/test/integration/factories/disks_factory.rb b/test/integration/factories/disks_factory.rb index 3fc39a8db5..16fd276888 100644 --- a/test/integration/factories/disks_factory.rb +++ b/test/integration/factories/disks_factory.rb @@ -9,6 +9,10 @@ def get(identity) @subject.get(identity, TEST_ZONE) end + def all + @subject.all(zone: TEST_ZONE) + end + def params { :name => resource_name, :zone_name => TEST_ZONE, diff --git a/test/integration/factories/forwarding_rules_factory.rb b/test/integration/factories/forwarding_rules_factory.rb index dc9064bd04..57351732ec 100644 --- a/test/integration/factories/forwarding_rules_factory.rb +++ b/test/integration/factories/forwarding_rules_factory.rb @@ -12,6 +12,10 @@ def cleanup @target_pools.cleanup end + def all + @subject.all(region: TEST_REGION) + end + def params { :name => resource_name, :port_range => "80-80", diff --git a/test/integration/factories/images_factory.rb b/test/integration/factories/images_factory.rb index b35dcdc2eb..81afb5e6c4 100644 --- a/test/integration/factories/images_factory.rb +++ b/test/integration/factories/images_factory.rb @@ -5,6 +5,10 @@ def initialize(example) super(Fog::Compute[:google].images, example) end + def get(identity) + @subject.get(identity, TEST_PROJECT) + end + def params { :name => resource_name, :raw_disk => { :source => TEST_RAW_DISK_SOURCE } } diff --git a/test/integration/factories/instance_group_manager_factory.rb b/test/integration/factories/instance_group_manager_factory.rb index dc46b9fcee..e9eb5c5493 100644 --- a/test/integration/factories/instance_group_manager_factory.rb +++ b/test/integration/factories/instance_group_manager_factory.rb @@ -12,6 +12,14 @@ def cleanup @instance_template.cleanup end + def get(identity) + @subject.get(identity, TEST_ZONE) + end + + def all + @subject.all(zone: TEST_ZONE) + end + def params { :name => resource_name, :zone => TEST_ZONE, diff --git a/test/integration/factories/instance_groups_factory.rb b/test/integration/factories/instance_groups_factory.rb index 99df352cfe..ce0eccc3c0 100644 --- a/test/integration/factories/instance_groups_factory.rb +++ b/test/integration/factories/instance_groups_factory.rb @@ -9,6 +9,10 @@ def get(identity) @subject.get(identity, TEST_ZONE) end + def all + @subject.all(zone: TEST_ZONE) + end + def params { :name => resource_name, :zone => TEST_ZONE } diff --git a/test/integration/factories/servers_factory.rb b/test/integration/factories/servers_factory.rb index 13f3644a1e..7ece4cadc4 100644 --- a/test/integration/factories/servers_factory.rb +++ b/test/integration/factories/servers_factory.rb @@ -13,6 +13,14 @@ def cleanup @disks.cleanup end + def get(identity) + @subject.get(identity, TEST_ZONE) + end + + def all + @subject.all(zone: TEST_ZONE) + end + def params { :name => resource_name, :zone => TEST_ZONE, diff --git a/test/integration/factories/subnetworks_factory.rb b/test/integration/factories/subnetworks_factory.rb index 370604900a..bec6172f23 100644 --- a/test/integration/factories/subnetworks_factory.rb +++ b/test/integration/factories/subnetworks_factory.rb @@ -13,6 +13,10 @@ def get(identity) @subject.get(identity, TEST_REGION) end + def all + @subject.all(region: TEST_REGION) + end + def params { :name => resource_name, :network => "default", diff --git a/test/integration/factories/target_instances_factory.rb b/test/integration/factories/target_instances_factory.rb index 7ac7f04536..e9fd9fb66d 100644 --- a/test/integration/factories/target_instances_factory.rb +++ b/test/integration/factories/target_instances_factory.rb @@ -12,6 +12,10 @@ def cleanup @servers.cleanup end + def all + @subject.all(zone: TEST_ZONE) + end + def params { :name => resource_name, :zone => TEST_ZONE, diff --git a/test/integration/factories/target_pools_factory.rb b/test/integration/factories/target_pools_factory.rb index 1f958b611f..610ea4ee28 100644 --- a/test/integration/factories/target_pools_factory.rb +++ b/test/integration/factories/target_pools_factory.rb @@ -15,6 +15,10 @@ def cleanup @http_health_checks.cleanup end + def all + @subject.all(region: TEST_REGION) + end + def params { :name => resource_name, :region => TEST_REGION, From 73888a21b5477b39bbf0aff65ca6fdbf02ba3101 Mon Sep 17 00:00:00 2001 From: Artem Yakimenko Date: Fri, 21 Sep 2018 15:12:19 +1000 Subject: [PATCH 24/27] Fix 404 in case image is requested with project + tiny refactor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Wrapped the scoped image request with the proper exception logic - Removed a code path that will never be executed I know this method is not pretty but I’ll address this separately to not induce scope creep on this PR --- lib/fog/compute/google/models/images.rb | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/lib/fog/compute/google/models/images.rb b/lib/fog/compute/google/models/images.rb index 44473f210b..0785f37950 100644 --- a/lib/fog/compute/google/models/images.rb +++ b/lib/fog/compute/google/models/images.rb @@ -47,24 +47,27 @@ def current def get(identity, project = nil) if project - # TODO: Remove this - see #405 - image = service.get_image(identity, project).to_h - image[:project] = project - return new(image) + begin + image = service.get_image(identity, project).to_h + # TODO: Remove response modification - see #405 + image[:project] = project + return new(image) + rescue ::Google::Apis::ClientError => e + raise e unless e.status_code == 404 + nil + end elsif identity - project.nil? ? projects = all_projects : projects = [project] + projects = all_projects projects.each do |proj| begin response = service.get_image(identity, proj).to_h - # TODO: Remove this - see #405 + # TODO: Remove response modification - see #405 response[:project] = proj image = response return new(image) rescue ::Google::Apis::ClientError => e next if e.status_code == 404 break - else - break end end # If nothing is found - return nil From 512850c1e3597fda8dc63650d29966ee5da8a018 Mon Sep 17 00:00:00 2001 From: Artem Yakimenko Date: Fri, 21 Sep 2018 15:12:45 +1000 Subject: [PATCH 25/27] Increase Images collection test coverage --- test/helpers/integration_test_helper.rb | 2 ++ .../compute/core_compute/test_images.rb | 21 +++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/test/helpers/integration_test_helper.rb b/test/helpers/integration_test_helper.rb index eda20fbdb3..dea2fc84fe 100644 --- a/test/helpers/integration_test_helper.rb +++ b/test/helpers/integration_test_helper.rb @@ -17,6 +17,8 @@ TEST_SIZE_GB = 10 TEST_MACHINE_TYPE = "n1-standard-1".freeze TEST_IMAGE = "debian-9-stretch-v20180611".freeze +TEST_IMAGE_PROJECT = "debian-cloud".freeze +TEST_IMAGE_FAMILY = "debian-9".freeze # XXX This depends on a public image in gs://fog-test-bucket; there may be a better way to do this # The image was created like so: https://cloud.google.com/compute/docs/images#export_an_image_to_google_cloud_storage diff --git a/test/integration/compute/core_compute/test_images.rb b/test/integration/compute/core_compute/test_images.rb index 72db4a20de..2331e98a29 100644 --- a/test/integration/compute/core_compute/test_images.rb +++ b/test/integration/compute/core_compute/test_images.rb @@ -8,4 +8,25 @@ def setup @subject = Fog::Compute[:google].images @factory = ImagesFactory.new(namespaced_name) end + + def test_get_specific_image + image = @subject.get(TEST_IMAGE) + refute_nil(image, "Images.get(#{TEST_IMAGE}) should not return nil") + assert_equal(image.family,TEST_IMAGE_FAMILY) + assert_equal(image.project,TEST_IMAGE_PROJECT) + end + + def test_get_specific_image_from_project + image = @subject.get(TEST_IMAGE,TEST_IMAGE_PROJECT) + refute_nil(image, "Images.get(#{TEST_IMAGE}) should not return nil") + assert_equal(image.family,TEST_IMAGE_FAMILY) + assert_equal(image.project,TEST_IMAGE_PROJECT) + end + + def test_get_from_family + image = @subject.get_from_family(TEST_IMAGE_FAMILY) + refute_nil(image,"Images.get_from_family(#{TEST_IMAGE_FAMILY}) should not return nil") + assert_equal(image.family,TEST_IMAGE_FAMILY) + assert_equal(image.project,TEST_IMAGE_PROJECT) + end end From 95499e4a70cdf717a76ea02f75cb58e152a2ad0d Mon Sep 17 00:00:00 2001 From: Artem Yakimenko Date: Fri, 21 Sep 2018 15:13:55 +1000 Subject: [PATCH 26/27] Check both codepaths when doing nonexistent resource tests --- test/helpers/test_collection.rb | 3 ++- test/integration/compute/core_compute/test_images.rb | 12 ++++++------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/test/helpers/test_collection.rb b/test/helpers/test_collection.rb index e892221622..909ab2c8cb 100644 --- a/test/helpers/test_collection.rb +++ b/test/helpers/test_collection.rb @@ -42,7 +42,8 @@ def test_lifecycle end def test_get_returns_nil_if_resource_does_not_exist - assert_nil @factory.get("fog-test-fake-identity") + assert_nil @factory.get("fog-test-fake-identity") if @factory.respond_to?(:get) + assert_nil @subject.get("fog-test-fake-identity") end def test_enumerable diff --git a/test/integration/compute/core_compute/test_images.rb b/test/integration/compute/core_compute/test_images.rb index 2331e98a29..801a9d1af2 100644 --- a/test/integration/compute/core_compute/test_images.rb +++ b/test/integration/compute/core_compute/test_images.rb @@ -12,21 +12,21 @@ def setup def test_get_specific_image image = @subject.get(TEST_IMAGE) refute_nil(image, "Images.get(#{TEST_IMAGE}) should not return nil") - assert_equal(image.family,TEST_IMAGE_FAMILY) - assert_equal(image.project,TEST_IMAGE_PROJECT) + assert_equal(image.family, TEST_IMAGE_FAMILY) + assert_equal(image.project, TEST_IMAGE_PROJECT) end def test_get_specific_image_from_project image = @subject.get(TEST_IMAGE,TEST_IMAGE_PROJECT) refute_nil(image, "Images.get(#{TEST_IMAGE}) should not return nil") - assert_equal(image.family,TEST_IMAGE_FAMILY) - assert_equal(image.project,TEST_IMAGE_PROJECT) + assert_equal(image.family, TEST_IMAGE_FAMILY) + assert_equal(image.project, TEST_IMAGE_PROJECT) end def test_get_from_family image = @subject.get_from_family(TEST_IMAGE_FAMILY) refute_nil(image,"Images.get_from_family(#{TEST_IMAGE_FAMILY}) should not return nil") - assert_equal(image.family,TEST_IMAGE_FAMILY) - assert_equal(image.project,TEST_IMAGE_PROJECT) + assert_equal(image.family, TEST_IMAGE_FAMILY) + assert_equal(image.project, TEST_IMAGE_PROJECT) end end From bf15cf068a74de5cabee36a998925917348edf35 Mon Sep 17 00:00:00 2001 From: Artem Yakimenko Date: Fri, 21 Sep 2018 16:16:10 +1000 Subject: [PATCH 27/27] Update changelog, remove debugger endpoints --- CHANGELOG.md | 50 ++++++++++++++++++- test/unit/compute/test_common_collections.rb | 1 - test/unit/dns/test_common_collections.rb | 1 - .../unit/monitoring/test_comon_collections.rb | 1 - test/unit/pubsub/test_common_collections.rb | 1 - test/unit/sql/test_common_collections.rb | 1 - .../storage/test_common_json_collections.rb | 1 - .../storage/test_common_xml_collections.rb | 1 - 8 files changed, 49 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c3b5e611e4..0fb819e951 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,15 +2,62 @@ All notable changes to this project will be documented in this file. The format is loosely based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). -## Next release +## 1.8.0 + +### User-facing + +#### Fixed + +- \#419 Locked down fog upstream dependencies to alleviate deprecation warnings + until they can be properly dealt with. [temikus] +- \#400 Small `%Collection%.get` and `%Collection%.all` behaviour fixes [temikus] + - `Fog::Google::SQL::Instances.get(nil)` no longer returns an invalid + `sql#instancesList` object. + - `Fog::Compute::Google::InstanceGroups.get` and `.all` methods now support more than + just `:filter` option, fixed `.all` output if zone wasn't provided. + - Fix a typo causing `Operations.get(region:REGION)` to fail. + - `Fog::Compute::Google::Images.get(IMAGE, PROJECT)`, now returns `nil` if image is not + found rather than throwing `Google::Apis::ClientError`. + +### Development changes + +#### Added + +- \#400 Additional test coverage [temikus] + - Expanded tests for `%Collection%.get` behavior - scoped requests (e.g. `get(zone:ZONE)`) + and their corresponding code paths are now also tested. + - Increase `Fog::Compute::Google::Images` integration test coverage. + - Unit tests now work without a `~/.fog` config file set up. + - Expanded unit test coverage. + +#### Changed + +- \#400 Refactored most compute `get()` and `all()` methods to common format. [temikus] + +#### Fixed + +- \#400 Removed the Travis Ruby 2.5 workaround. [temikus] + +## 1.7.1 + +### User-facing + +#### Fixed + +- \#412 Fixed `Fog::Storage::GoogleXML::GetObjectHttpUrl#get_object_http_url` + request + +## 1.7.0 ### User-facing #### Added +- \#409 Support query parameters in `Fog::Storage::Google` GET requests [stanhu] - \#394 Add some helper methods to `Fog::Compute::Google::Server` [temikus] - `.private_ip_address` - `.stopped?` +- \#375 Add timeout options to `Fog::Storage::GoogleJSON` client [dosuken123] #### Changed @@ -24,6 +71,7 @@ The format is loosely based on [Keep a Changelog](http://keepachangelog.com/en/1 #### Added +- \#409 Expand `Fog::Storage::Google` unit tests [stanhu] - \#370 Introducing test coverage back, integrating with codecov.io [temikus] - \#373 Increase integration test coverage. [temikus] - Add Firewall factory and tests. diff --git a/test/unit/compute/test_common_collections.rb b/test/unit/compute/test_common_collections.rb index 0e18e49ba5..972b08ac03 100644 --- a/test/unit/compute/test_common_collections.rb +++ b/test/unit/compute/test_common_collections.rb @@ -1,5 +1,4 @@ require "helpers/test_helper" -require "pry" class UnitTestCollections < MiniTest::Test def setup diff --git a/test/unit/dns/test_common_collections.rb b/test/unit/dns/test_common_collections.rb index dd8ff992c2..7a1848bdf8 100644 --- a/test/unit/dns/test_common_collections.rb +++ b/test/unit/dns/test_common_collections.rb @@ -1,5 +1,4 @@ require "helpers/test_helper" -require "pry" class UnitTestDNSCollections < MiniTest::Test def setup diff --git a/test/unit/monitoring/test_comon_collections.rb b/test/unit/monitoring/test_comon_collections.rb index 9ee054b306..9f8870a28f 100644 --- a/test/unit/monitoring/test_comon_collections.rb +++ b/test/unit/monitoring/test_comon_collections.rb @@ -1,5 +1,4 @@ require "helpers/test_helper" -require "pry" class UnitTestMonitoringCollections < MiniTest::Test def setup diff --git a/test/unit/pubsub/test_common_collections.rb b/test/unit/pubsub/test_common_collections.rb index 828f853f18..6628be2872 100644 --- a/test/unit/pubsub/test_common_collections.rb +++ b/test/unit/pubsub/test_common_collections.rb @@ -1,5 +1,4 @@ require "helpers/test_helper" -require "pry" class UnitTestPubsubCollections < MiniTest::Test def setup diff --git a/test/unit/sql/test_common_collections.rb b/test/unit/sql/test_common_collections.rb index 997ad17ee6..66e5711f14 100644 --- a/test/unit/sql/test_common_collections.rb +++ b/test/unit/sql/test_common_collections.rb @@ -1,5 +1,4 @@ require "helpers/test_helper" -require "pry" class UnitTestSQLCollections < MiniTest::Test def setup diff --git a/test/unit/storage/test_common_json_collections.rb b/test/unit/storage/test_common_json_collections.rb index 8c3a6b7a34..54ed074add 100644 --- a/test/unit/storage/test_common_json_collections.rb +++ b/test/unit/storage/test_common_json_collections.rb @@ -1,5 +1,4 @@ require "helpers/test_helper" -require "pry" class UnitTestStorageJSONCollections < MiniTest::Test def setup diff --git a/test/unit/storage/test_common_xml_collections.rb b/test/unit/storage/test_common_xml_collections.rb index e934d08a77..60bc88ba12 100644 --- a/test/unit/storage/test_common_xml_collections.rb +++ b/test/unit/storage/test_common_xml_collections.rb @@ -1,5 +1,4 @@ require "helpers/test_helper" -require "pry" class UnitTestStorageXMLCollections < MiniTest::Test def setup