Skip to content
This repository has been archived by the owner on Jun 19, 2022. It is now read-only.

Commit

Permalink
address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
Nicolas Lopez committed Feb 18, 2020
1 parent 43a2d82 commit 9b13ee8
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 8 deletions.
2 changes: 1 addition & 1 deletion test/e2e/e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ func TestBrokerWithPubSubChannel(t *testing.T) {
BrokerWithPubSubChannelTestImpl(t)
}

// TestBrokerWithPubSubChannel tests we can knock a Knative Service from a broker with PubSub Channel from a CloudPubSubSource.
// TestCloudPubSubSourceBrokerWithPubSubChannel tests we can knock a Knative Service from a broker with PubSub Channel from a CloudPubSubSource.
func TestCloudPubSubSourceBrokerWithPubSubChannel(t *testing.T) {
cancel := logstream.Start(t)
defer cancel()
Expand Down
7 changes: 6 additions & 1 deletion test/e2e/lib/pubsub.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,12 @@ import (
pkgmetrics "knative.dev/pkg/metrics"
)

func MakePubSubOrDie(t *testing.T, client *Client, gvk metav1.GroupVersionKind, psName, targetName, topicName string, so ...kngcptesting.CloudPubSubSourceOption) {
func MakePubSubOrDie(t *testing.T,
client *Client,
gvk metav1.GroupVersionKind,
psName, targetName, topicName string,
so ...kngcptesting.CloudPubSubSourceOption,
) {
so = append(so, kngcptesting.WithCloudPubSubSourceSink(gvk, targetName))
so = append(so, kngcptesting.WithCloudPubSubSourceTopic(topicName))
eventsPubsub := kngcptesting.NewCloudPubSubSource(psName, client.Namespace, so...)
Expand Down
35 changes: 29 additions & 6 deletions test/e2e/test_broker_pubsub.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,14 @@ func BrokerWithPubSubChannelTestImpl(t *testing.T) {
client := lib.Setup(t, true)
defer lib.TearDown(client)

u := createBrokerWithPubSubChannel(t, client, brokerName, dummyTriggerName, kserviceName, respTriggerName, targetName)
u := createBrokerWithPubSubChannel(t,
client,
brokerName,
dummyTriggerName,
kserviceName,
respTriggerName,
targetName,
)

// Just to make sure all resources are ready.
time.Sleep(5 * time.Second)
Expand Down Expand Up @@ -104,13 +111,27 @@ func PubSubSourceBrokerWithPubSubChannelTestImpl(t *testing.T) {
client := lib.Setup(t, true)
defer lib.TearDown(client)

u := createBrokerWithPubSubChannel(t, client, brokerName, dummyTriggerName, kserviceName, respTriggerName, targetName)
u := createBrokerWithPubSubChannel(t,
client,
brokerName,
dummyTriggerName,
kserviceName,
respTriggerName,
targetName,
)
var url apis.URL = apis.URL(u)
// Just to make sure all resources are ready.
time.Sleep(5 * time.Second)

// Create the PubSub source.
lib.MakePubSubOrDie(t, client, lib.ServiceGVK, psName, targetName, topicName, kngcptesting.WithCloudPubSubSourceSinkURI(&url))
lib.MakePubSubOrDie(t,
client,
lib.ServiceGVK,
psName,
targetName,
topicName,
kngcptesting.WithCloudPubSubSourceSinkURI(&url),
)

topic := lib.GetTopic(t, topicName)

Expand All @@ -131,11 +152,13 @@ func PubSubSourceBrokerWithPubSubChannelTestImpl(t *testing.T) {
t.Error("resp event didn't hit the target pod")
t.Failed()
}

// TODO(nlopezgi): define if we want to assert StackDriver metrics here. CloudPubSubSourceWithTargetTestImpl can do so, but the test is currently disabled, so its unclear whether we want to replicate that functionality here.
// TODO(nlopezgi): assert StackDriver metrics after https://github.com/google/knative-gcp/issues/317 is resolved
}

func createBrokerWithPubSubChannel(t *testing.T, client *lib.Client, brokerName, dummyTriggerName, kserviceName, respTriggerName, targetName string) url.URL {
func createBrokerWithPubSubChannel(t *testing.T,
client *lib.Client,
brokerName, dummyTriggerName, kserviceName, respTriggerName, targetName string,
) url.URL {
// Create a new Broker.
// TODO(chizhg): maybe we don't need to create these RBAC resources as they will now be automatically created?
client.Core.CreateRBACResourcesForBrokers()
Expand Down

0 comments on commit 9b13ee8

Please sign in to comment.