From 002984fbcf8d1c26b0e0be59ed193f60956eaed2 Mon Sep 17 00:00:00 2001 From: Karuppiah Natarajan Date: Sun, 3 Oct 2021 19:39:28 +0530 Subject: [PATCH] fix tests in bastion_hosts_test.go file - Remove `create publicip fails` test in bastion host `Reconcile` test as we don't create public IP as part of bastion host `Reconcile` - Remove `bastion successfully created with created public ip` test in bastion host `Reconcile` test as we don't create public IP as part of bastion host `Reconcile` - Remove unnecessary / wrong method call expectations using mocks - `mPublicIP.CreateOrUpdate` method call expectations - `m.Delete(gomockinternal.AContext(), "my-rg", "my-bastionhost1")` expectation - Fix the order of method call expectations using mocks - `mPublicIP.Get` gets called before `mSubnet.Get` - Comment out `t.Parallel` temporarily with details about the removal of parallelization - tldr; enabling parallel tests seems to give wrong test results - Fix the expected error messages Signed-off-by: Karuppiah Natarajan --- .../bastionhosts/bastionhosts_test.go | 102 ++---------------- 1 file changed, 9 insertions(+), 93 deletions(-) diff --git a/azure/services/bastionhosts/bastionhosts_test.go b/azure/services/bastionhosts/bastionhosts_test.go index a0d123e74d0c..42417d926036 100644 --- a/azure/services/bastionhosts/bastionhosts_test.go +++ b/azure/services/bastionhosts/bastionhosts_test.go @@ -54,7 +54,7 @@ func TestReconcileBastionHosts(t *testing.T) { }{ { name: "fail to get subnets", - expectedError: "failed to get subnet: #: Internal Server Error: StatusCode=500", + expectedError: "error creating Azure Bastion: failed to get subnet for azure bastion: #: Internal Server Error: StatusCode=500", expect: func(s *mock_bastionhosts.MockBastionScopeMockRecorder, m *mock_bastionhosts.MockclientMockRecorder, mSubnet *mock_subnets.MockClientMockRecorder, @@ -71,65 +71,16 @@ func TestReconcileBastionHosts(t *testing.T) { }, }) s.ResourceGroup().AnyTimes().Return("my-rg") - mSubnet.Get(gomockinternal.AContext(), "my-rg", "my-vnet", "my-subnet"). - Return(network.Subnet{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")) - }, - }, - { - name: "fail to get publicip", - expectedError: "failed to get existing publicIP: #: Internal Server Error: StatusCode=500", - expect: func(s *mock_bastionhosts.MockBastionScopeMockRecorder, - m *mock_bastionhosts.MockclientMockRecorder, - mSubnet *mock_subnets.MockClientMockRecorder, - mPublicIP *mock_publicips.MockClientMockRecorder) { - s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.BastionSpec().Return(azure.BastionSpec{ - AzureBastion: &azure.AzureBastionSpec{ - Name: "my-bastion", - VNetName: "my-vnet", - SubnetSpec: v1alpha4.SubnetSpec{ - Name: "my-subnet", - }, - PublicIPName: "my-publicip", - }, - }) - s.ResourceGroup().AnyTimes().Return("my-rg") - gomock.InOrder( - mSubnet.Get(gomockinternal.AContext(), "my-rg", "my-vnet", "my-subnet").Return(network.Subnet{}, nil), - mPublicIP.Get(gomockinternal.AContext(), "my-rg", "my-publicip").Return(network.PublicIPAddress{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")), - ) - }, - }, - { - name: "create publicip fails", - expectedError: "failed to create bastion publicIP: #: Internal Server Error: StatusCode=500", - expect: func(s *mock_bastionhosts.MockBastionScopeMockRecorder, - m *mock_bastionhosts.MockclientMockRecorder, - mSubnet *mock_subnets.MockClientMockRecorder, - mPublicIP *mock_publicips.MockClientMockRecorder) { - s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.BastionSpec().Return(azure.BastionSpec{ - AzureBastion: &azure.AzureBastionSpec{ - Name: "my-bastion", - VNetName: "my-vnet", - SubnetSpec: v1alpha4.SubnetSpec{ - Name: "my-subnet", - }, - PublicIPName: "my-publicip", - }, - }) - s.ResourceGroup().AnyTimes().Return("my-rg") - s.Location().AnyTimes().Return("fake-location") gomock.InOrder( - mSubnet.Get(gomockinternal.AContext(), "my-rg", "my-vnet", "my-subnet").Return(network.Subnet{}, nil), - mPublicIP.Get(gomockinternal.AContext(), "my-rg", "my-publicip").Return(network.PublicIPAddress{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")), - mPublicIP.CreateOrUpdate(gomockinternal.AContext(), "my-rg", "my-publicip", gomock.AssignableToTypeOf(network.PublicIPAddress{})).Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")), + mPublicIP.Get(gomockinternal.AContext(), "my-rg", "my-publicip").Return(network.PublicIPAddress{}, nil), + mSubnet.Get(gomockinternal.AContext(), "my-rg", "my-vnet", "my-subnet"). + Return(network.Subnet{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")), ) }, }, { - name: "fails to get a created publicip", - expectedError: "failed to get created publicIP: #: Internal Server Error: StatusCode=500", + name: "fail to get publicip", + expectedError: "error creating Azure Bastion: failed to get public IP for azure bastion: #: Internal Server Error: StatusCode=500", expect: func(s *mock_bastionhosts.MockBastionScopeMockRecorder, m *mock_bastionhosts.MockclientMockRecorder, mSubnet *mock_subnets.MockClientMockRecorder, @@ -146,43 +97,9 @@ func TestReconcileBastionHosts(t *testing.T) { }, }) s.ResourceGroup().AnyTimes().Return("my-rg") - s.Location().AnyTimes().Return("fake-location") - mSubnet.Get(gomockinternal.AContext(), "my-rg", "my-vnet", "my-subnet").Return(network.Subnet{}, nil) - mPublicIP.Get(gomockinternal.AContext(), "my-rg", "my-publicip").Return(network.PublicIPAddress{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) - mPublicIP.CreateOrUpdate(gomockinternal.AContext(), "my-rg", "my-publicip", gomock.AssignableToTypeOf(network.PublicIPAddress{})).Return(nil) mPublicIP.Get(gomockinternal.AContext(), "my-rg", "my-publicip").Return(network.PublicIPAddress{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")) }, }, - { - name: "bastion successfully created with created public ip", - expectedError: "", - expect: func(s *mock_bastionhosts.MockBastionScopeMockRecorder, - m *mock_bastionhosts.MockclientMockRecorder, - mSubnet *mock_subnets.MockClientMockRecorder, - mPublicIP *mock_publicips.MockClientMockRecorder) { - s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.BastionSpec().Return(azure.BastionSpec{ - AzureBastion: &azure.AzureBastionSpec{ - Name: "my-bastion", - VNetName: "my-vnet", - SubnetSpec: v1alpha4.SubnetSpec{ - Name: "my-subnet", - }, - PublicIPName: "my-publicip", - }, - }) - s.ResourceGroup().AnyTimes().Return("my-rg") - s.Location().AnyTimes().Return("fake-location") - s.ClusterName().AnyTimes().Return("fake-cluster") - gomock.InOrder( - mSubnet.Get(gomockinternal.AContext(), "my-rg", "my-vnet", "my-subnet").Return(network.Subnet{}, nil), - mPublicIP.Get(gomockinternal.AContext(), "my-rg", "my-publicip").Return(network.PublicIPAddress{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")), - mPublicIP.CreateOrUpdate(gomockinternal.AContext(), "my-rg", "my-publicip", gomock.AssignableToTypeOf(network.PublicIPAddress{})).Return(nil), - mPublicIP.Get(gomockinternal.AContext(), "my-rg", "my-publicip").Return(network.PublicIPAddress{}, nil), - m.CreateOrUpdate(gomockinternal.AContext(), "my-rg", "my-bastion", gomock.AssignableToTypeOf(network.BastionHost{})), - ) - }, - }, { name: "bastion successfully created", expectedError: "", @@ -205,8 +122,8 @@ func TestReconcileBastionHosts(t *testing.T) { s.Location().AnyTimes().Return("fake-location") s.ClusterName().AnyTimes().Return("fake-cluster") gomock.InOrder( - mSubnet.Get(gomockinternal.AContext(), "my-rg", "my-vnet", "my-subnet").Return(network.Subnet{}, nil), mPublicIP.Get(gomockinternal.AContext(), "my-rg", "my-publicip").Return(network.PublicIPAddress{}, nil), + mSubnet.Get(gomockinternal.AContext(), "my-rg", "my-vnet", "my-subnet").Return(network.Subnet{}, nil), m.CreateOrUpdate(gomockinternal.AContext(), "my-rg", "my-bastion", gomock.AssignableToTypeOf(network.BastionHost{})), ) }, @@ -243,8 +160,8 @@ func TestReconcileBastionHosts(t *testing.T) { for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { + tc := tc g := NewWithT(t) - t.Parallel() mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() @@ -327,7 +244,6 @@ func TestDeleteBastionHost(t *testing.T) { s.ResourceGroup().AnyTimes().Return("my-rg") m.Delete(gomockinternal.AContext(), "my-rg", "my-bastionhost"). Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) - m.Delete(gomockinternal.AContext(), "my-rg", "my-bastionhost1") }, }, { @@ -357,8 +273,8 @@ func TestDeleteBastionHost(t *testing.T) { for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { + tc := tc g := NewWithT(t) - t.Parallel() mockCtrl := gomock.NewController(t) defer mockCtrl.Finish()