Azure service bus conf tests (#614)

* Add azure service bus conformance tests
Bubble up error on required env var not set

* Update common.go

* Fix running output bindings with errors

* update readme

Co-authored-by: Artur Souza <artursouza.ms@outlook.com>

.github/workflows/conformance.yml

@@ -34,32 +34,35 @@ jobs:
         component:
         - output-binding.azure.blobstorage
         - output-binding.azure.storagequeues
+        - pubsub.azure.servicebus
         - pubsub.redis
         - secretstores.localenv
+        - secretstores.localfile
         - state.cosmosdb
         - state.mongodb
         - state.redis
         include:
+        # Unfortunately, Azure secrets can't have underscores in
+        # names, while environment variables with hyphens ('-') are
+        # troublesome.
+        #
+        # We work around here by leveraging the fact that
+        # environment variable names are case sensitive, so
+        # CamelCase would still work.
+        #
+        # That is slightly better than something like
+        # AZURECOSMOSDBMASTERKEY, which is extremely hard to read
+        # and errorprone.
+        #
+        # Only list the secrets you need for the component.
         - component: state.cosmosdb
-          # Unfortunately, Azure secrets can't have underscores in
-          # names, while environment variables with hyphens ('-') are
-          # troublesome.
-          #
-          # We work around here by leveraging the fact that
-          # environment variable names are case sensitive, so
-          # CamelCase would still work.
-          #
-          # That is slightly better than something like
-          # AZURECOSMOSDBMASTERKEY, which is extremely hard to read
-          # and errorprone.
-          #
-          # Only list the secrets you need for this component.
           required-secrets: AzureCosmosDBMasterKey,AzureCosmosDBUrl,AzureCosmosDB,AzureCosmosDBCollection
+        - component: pubsub.azure.servicebus
+          required-secrets: AzureServiceBusConnectionString
         - component: output-binding.azure.blobstorage
           required-secrets: AzureBlobStorageAccessKey,AzureBlobStorageAccount,AzureBlobStorageContainer
         - component: output-binding.azure.storagequeues
           required-secrets: AzureBlobStorageAccessKey,AzureBlobStorageAccount,AzureBlobStorageQueue
-
     steps:
     - name: Check out code onto GOPATH
       uses: actions/checkout@v2
@@ -110,7 +113,7 @@ jobs:
 
         echo "Running tests for Test${KIND_UPPER}Conformance/${KIND}/${NAME} ... "
 
-        go test -v -tags=conftests -count=1 ./tests/... \
+        go test -v -tags=conftests -count=1 ./tests/conformance/... \
           --run="Test${KIND_UPPER}Conformance/${KIND}/${NAME}" 2>&1 | tee output.log
 
         # Fail the step if we found no test to run

tests/config/pubsub/azure/servicebus/pubsub.yml

@@ -0,0 +1,12 @@
+apiVersion: dapr.io/v1alpha1
+kind: Component
+metadata:
+  name: azure-servicebus
+spec:
+  type: pubsub.azure.servicebus
+  version: v1
+  metadata:
+  - name: connectionString
+    value: ${{AzureServiceBusConnectionString}}
+  - name: consumerID
+    value: "testConsumer"

tests/config/pubsub/tests.yml

@@ -1,4 +1,11 @@
 componentType: pubsub
 components:
+  - component: azure.servicebus
+    allOperations: true
+    config:
+      pubsubName: azure-servicebus
+      testTopicName: dapr-conf-test
+      maxReadDuration: 1000
   - component: redis
-    allOperations: true
+    allOperations: true
+  

tests/conformance/README.md

@@ -5,14 +5,23 @@
 1. `tests/` directory contains the configuration and the test definition for conformance tests.
 2. All the conformance tests are within the `tests/conformance` directory.
 3. All the configurations are in the `tests/config` directory.
-4. Each of the component specific `component` definition are in their specific `component type` folder in the `tests/config` folder. For eg: `redis` statestore component definition within `state` directory. And the other component types are `state`, `secretstores`, `pubsub` and `bindings`.
+4. Each of the component specific `component` definition are in their specific `component type` folder in the `tests/config` folder. For eg: `redis` statestore component definition within `state` directory. And the other component types are `state`, `secretstores`, `pubsub`. Cloud specific components will be within their own `cloud` directory within the `component type` folder eg: `pubsub/azure/servicebus`.
 5. Similar to the component definitions, each component type has its own set of the conformance tests definitions.
-6. Each component type contains a `tests.yml` definition that defines the component to be tested along with component specific test configuration.
+6. Each `component type` contains a `tests.yml` definition that defines the component to be tested along with component specific test configuration. Nested folder names have their `/` in path replaced by `.` in the component name in `tests.yml` eg: `azure/servicebus` should be `azure.servicebus`
 7. All the tests configurations are defined in `common.go` file.
-8. Each component type has its own `_test` file to trigger the conformance tests.
+8. Each `component type` has its own `_test` file to trigger the conformance tests.
+9. Each test added will also need to be added to the `conformance.yml` workflow file. 
 
 ## Running conformance tests
 1. Test test setup is independent of the test run.
 2. Run Redis with 6379 exposed locally.
-3. Run `make conf-tests` to run the conformance tests locally.
+3. Run Mongodb locally.
+4. Run all conformance tests with `make conf-tests`.
 > Note Some conformance tests require credentials in the form of environment variables. For examples Azure CosmosDB conformance tests will need to have Azure CosmosDB credentials. You will need to supply them to make these tests pass.
+5. To run specific tests, run:
+```bash
+# TEST_NAME can be TestPubsubConformance, TestStateConformance, TestSecretStoreConformance or TestOutputBindingConformance
+# COMPONENT_TYPE is the type of the component can be pubsub, state, output-binding, secretstores
+# COMPONENT_NAME is the component name from the tests.yml file eg: azure.servicebus, redis, mongodb etc.
+go test -v -tags=conftests -count=1 ./tests/conformance -run="${TEST_NAME}/${COMPONENT_TYPE}/${COMPONENT_NAME}"
+```

tests/conformance/common.go

@@ -100,7 +100,7 @@ func LookUpEnv(key string) string {
 	return ""
 }
 
-func ConvertMetadataToProperties(items []v1alpha1.MetadataItem) map[string]string {
+func ConvertMetadataToProperties(items []v1alpha1.MetadataItem) (map[string]string, error) {
 	properties := map[string]string{}
 	for _, c := range items {
 		val := c.Value.String()
@@ -108,14 +108,15 @@ func ConvertMetadataToProperties(items []v1alpha1.MetadataItem) map[string]strin
 			// look up env var with that name. remove ${{}} and space
 			k := strings.TrimSpace(strings.TrimSuffix(strings.TrimPrefix(val, "${{"), "}}"))
 			v := LookUpEnv(k)
-			if v != "" {
-				val = v
+			if v == "" {
+				return map[string]string{}, fmt.Errorf("required env var is not set %s", k)
 			}
+			val = v
 		}
 		properties[c.Name] = val
 	}
 
-	return properties
+	return properties, nil
 }
 
 // nolint:gosec
@@ -161,14 +162,14 @@ func decodeYaml(b []byte) (TestConfiguration, error) {
 	return testConfig, nil
 }
 
-func (tc *TestConfiguration) loadComponentsAndProperties(t *testing.T, filepath string) map[string]string {
+func (tc *TestConfiguration) loadComponentsAndProperties(t *testing.T, filepath string) (map[string]string, error) {
 	comps, err := LoadComponents(filepath)
 	assert.Nil(t, err)
-	assert.Equal(t, 1, len(comps)) // We only expect a single component per state store but on
+	assert.Equal(t, 1, len(comps)) // We only expect a single component per file
 	c := comps[0]
-	props := ConvertMetadataToProperties(c.Spec.Metadata)
+	props, err := ConvertMetadataToProperties(c.Spec.Metadata)
 
-	return props
+	return props, err
 }
 
 func convertComponentNameToPath(componentName string) string {
@@ -179,35 +180,56 @@ func convertComponentNameToPath(componentName string) string {
 	return componentName
 }
 
-func (tc *TestConfiguration) Run(t *testing.T) {
+func (tc *TestConfiguration) Run(t *testing.T) []error {
+	var errs []error
 	// For each component in the tests file run the conformance test
 	for _, comp := range tc.Components {
 		componentConfigPath := convertComponentNameToPath(comp.Component)
 		switch tc.ComponentType {
 		case "state":
 			filepath := fmt.Sprintf("../config/state/%s", componentConfigPath)
-			props := tc.loadComponentsAndProperties(t, filepath)
+			props, err := tc.loadComponentsAndProperties(t, filepath)
+			if err != nil {
+				errs = append(errs, fmt.Errorf("error running conformance test for %s: %w", comp.Component, err))
+
+				continue
+			}
 			store := loadStateStore(comp)
 			assert.NotNil(t, store)
 			storeConfig := conf_state.NewTestConfig(comp.Component, comp.AllOperations, comp.Operations, comp.Config)
 			conf_state.ConformanceTests(t, props, store, storeConfig)
 		case "secretstores":
 			filepath := fmt.Sprintf("../config/secretstores/%s", componentConfigPath)
-			props := tc.loadComponentsAndProperties(t, filepath)
+			props, err := tc.loadComponentsAndProperties(t, filepath)
+			if err != nil {
+				errs = append(errs, fmt.Errorf("error running conformance test for %s: %w", comp.Component, err))
+
+				continue
+			}
 			store := loadSecretStore(comp)
 			assert.NotNil(t, store)
 			storeConfig := conf_secret.NewTestConfig(comp.Component, comp.AllOperations, comp.Operations)
 			conf_secret.ConformanceTests(t, props, store, storeConfig)
 		case "pubsub":
 			filepath := fmt.Sprintf("../config/pubsub/%s", componentConfigPath)
-			props := tc.loadComponentsAndProperties(t, filepath)
+			props, err := tc.loadComponentsAndProperties(t, filepath)
+			if err != nil {
+				errs = append(errs, fmt.Errorf("error running conformance test for %s: %w", comp.Component, err))
+
+				continue
+			}
 			pubsub := loadPubSub(comp)
 			assert.NotNil(t, pubsub)
 			pubsubConfig := conf_pubsub.NewTestConfig(comp.Component, comp.AllOperations, comp.Operations, comp.Config)
 			conf_pubsub.ConformanceTests(t, props, pubsub, pubsubConfig)
 		case "output-binding":
 			filepath := fmt.Sprintf("../config/bindings/%s", componentConfigPath)
-			props := tc.loadComponentsAndProperties(t, filepath)
+			props, err := tc.loadComponentsAndProperties(t, filepath)
+			if err != nil {
+				errs = append(errs, fmt.Errorf("error running conformance test for %s: %w", comp.Component, err))
+
+				continue
+			}
 			binding := loadOutputBindings(comp)
 			assert.NotNil(t, binding)
 			bindingsConfig := conf_output_bindings.NewTestConfig(comp.Component, comp.AllOperations, comp.Operations)
@@ -216,6 +238,8 @@ func (tc *TestConfiguration) Run(t *testing.T) {
 			assert.Failf(t, "unknown component type %s", tc.ComponentType)
 		}
 	}
+
+	return errs
 }
 
 func loadPubSub(tc TestComponent) pubsub.PubSub {
@@ -223,7 +247,7 @@ func loadPubSub(tc TestComponent) pubsub.PubSub {
 	switch tc.Component {
 	case redis:
 		pubsub = p_redis.NewRedisStreams(testLogger)
-	case "azure-servicebus":
+	case "azure.servicebus":
 		pubsub = p_servicebus.NewAzureServiceBus(testLogger)
 	default:
 		return nil

tests/conformance/common_test.go

@@ -68,13 +68,23 @@ func TestConvertMetadataToProperties(t *testing.T) {
 			},
 		},
 	}
-	os.Setenv("CONF_TEST_KEY", "testval")
-	defer os.Unsetenv("CONF_TEST_KEY")
-	resp := ConvertMetadataToProperties(items)
-	assert.NotNil(t, resp)
-	assert.Equal(t, 2, len(resp))
-	assert.Equal(t, "test", resp["test_key"])
-	assert.Equal(t, "testval", resp["env_var_sub"])
+	t.Run("env var set", func(t *testing.T) {
+		os.Setenv("CONF_TEST_KEY", "testval")
+		defer os.Unsetenv("CONF_TEST_KEY")
+		resp, err := ConvertMetadataToProperties(items)
+		assert.NoError(t, err)
+		assert.NotNil(t, resp)
+		assert.Equal(t, 2, len(resp))
+		assert.Equal(t, "test", resp["test_key"])
+		assert.Equal(t, "testval", resp["env_var_sub"])
+	})
+
+	t.Run("env var not set", func(t *testing.T) {
+		resp, err := ConvertMetadataToProperties(items)
+		assert.NotNil(t, err)
+		assert.NotNil(t, resp)
+		assert.Equal(t, 0, len(resp))
+	})
 }
 
 func TestConvertComponentNameToPath(t *testing.T) {

tests/conformance/output_bindings_test.go

@@ -12,5 +12,11 @@ func TestOutputBindingConformance(t *testing.T) {
 	tc, err := NewTestConfiguration("../config/bindings/output_tests.yml")
 	assert.NoError(t, err)
 	assert.NotNil(t, tc)
-	tc.Run(t)
+	errs := tc.Run(t)
+	if len(errs) != 0 {
+		for _, err = range errs {
+			t.Log(err)
+		}
+		assert.Fail(t, "some of tests failed")
+	}
 }

tests/conformance/pubsub/pubsub.go

@@ -73,14 +73,11 @@ func NewTestConfig(componentName string, allOperations bool, operations []string
 	return tc
 }
 
-func ConformanceTests(t *testing.T, props map[string]string, pubusub pubsub.PubSub, config TestConfig) {
-	// Properly close connection to pubsub
-	defer pubusub.Close()
-
+func ConformanceTests(t *testing.T, props map[string]string, ps pubsub.PubSub, config TestConfig) {
 	actualReadCount := 0
 	if config.CommonConfig.HasOperation("init") {
 		t.Run(config.GetTestName("init"), func(t *testing.T) {
-			err := pubusub.Init(pubsub.Metadata{
+			err := ps.Init(pubsub.Metadata{
 				Properties: props,
 			})
 			assert.NoError(t, err, "expected no error on setting up pubsub")
@@ -89,7 +86,7 @@ func ConformanceTests(t *testing.T, props map[string]string, pubusub pubsub.PubS
 
 	if config.HasOperation("subscribe") {
 		t.Run(config.GetTestName("subscribe"), func(t *testing.T) {
-			err := pubusub.Subscribe(pubsub.SubscribeRequest{
+			err := ps.Subscribe(pubsub.SubscribeRequest{
 				Topic:    config.testTopicName,
 				Metadata: config.subscribeMetadata,
 			}, func(_ *pubsub.NewMessage) error {
@@ -105,7 +102,7 @@ func ConformanceTests(t *testing.T, props map[string]string, pubusub pubsub.PubS
 		t.Run(config.GetTestName("publish"), func(t *testing.T) {
 			for k := 0; k < config.messageCount; k++ {
 				data := []byte("message-" + strconv.Itoa(k))
-				err := pubusub.Publish(&pubsub.PublishRequest{
+				err := ps.Publish(&pubsub.PublishRequest{
 					Data:       data,
 					PubsubName: config.pubsubName,
 					Topic:      config.testTopicName,
@@ -123,4 +120,10 @@ func ConformanceTests(t *testing.T, props map[string]string, pubusub pubsub.PubS
 			assert.LessOrEqual(t, config.messageCount, actualReadCount, "expected to read %v messages", config.messageCount)
 		})
 	}
+
+	t.Run(config.GetTestName("close pubsub"), func(t *testing.T) {
+		// Properly close connection to pubsub
+		// This is needed inside t.Run because of the way the individual tests are run based on name filtering in the conformance test workflow.
+		ps.Close()
+	})
 }

tests/conformance/pubsub_test.go

@@ -12,5 +12,11 @@ func TestPubsubConformance(t *testing.T) {
 	tc, err := NewTestConfiguration("../config/pubsub/tests.yml")
 	assert.NoError(t, err)
 	assert.NotNil(t, tc)
-	tc.Run(t)
+	errs := tc.Run(t)
+	if len(errs) != 0 {
+		for _, err = range errs {
+			t.Log(err)
+		}
+		assert.Fail(t, "some of tests failed")
+	}
 }

tests/conformance/secretstores_test.go

@@ -12,5 +12,11 @@ func TestSecretStoreConformance(t *testing.T) {
 	tc, err := NewTestConfiguration("../config/secretstores/tests.yml")
 	assert.NoError(t, err)
 	assert.NotNil(t, tc)
-	tc.Run(t)
+	errs := tc.Run(t)
+	if len(errs) != 0 {
+		for _, err = range errs {
+			t.Log(err)
+		}
+		assert.Fail(t, "some of tests failed")
+	}
 }

tests/conformance/state_test.go

@@ -12,5 +12,11 @@ func TestStateConformance(t *testing.T) {
 	tc, err := NewTestConfiguration("../config/state/tests.yml")
 	assert.NoError(t, err)
 	assert.NotNil(t, tc)
-	tc.Run(t)
+	errs := tc.Run(t)
+	if len(errs) != 0 {
+		for _, err = range errs {
+			t.Log(err)
+		}
+		assert.Fail(t, "some of tests failed")
+	}
 }