cleanup

Author: lychung83

monitoring/exporter/stackdriver/data_test.go

@@ -89,8 +89,8 @@ var (
 		Aggregation: view.Sum(),
 	}
 
-	// To make verification easy, we require all valid rows should int64 values and all of them
-	// must be distinct.
+	// To make verification easy, we require all valid rows should have int64 values and all of
+	// them must be distinct.
 	view1row1 = &view.Row{
 		Tags: nil,
 		Data: &view.SumData{Value: 1},

monitoring/exporter/stackdriver/mock_check_test.go

@@ -30,21 +30,23 @@ import (
 // This file defines various mocks for testing, and checking functions for mocked data. We mock
 // metric client and bundler because their actions involves RPC calls or non-deterministic behavior.
 
+// Following data are used to store various data generated by exporters' activity. They are used by
+// each test to verify intended behavior. Each test should call testDataInit() to clear these data.
 var (
-	// bundlerRec records project ID of given bundler.
+	// errStorage records all errors and associated RowData objects reported by exporter.
+	errStorage []errRowData
+	// bndlerProjMap records project ID of given bundler.
 	bndlerProjMap map[*bundler.Bundler]string
-	// projRds saves all RowData objects passed to addToBundler call by project ID.
+	// projRds saves all RowData objects passed to addToBundler call by project ID. Since a
+	// value of a map is not addressable, we save the pointer to the slice.
 	projRds map[string]*[]*RowData
-
 	// timeSeriesReqs saves all incoming requests for creating time series.
 	timeSeriesReqs []*mpb.CreateTimeSeriesRequest
 	// timeSeriesResults holds predefined error values to be returned by mockCreateTimeSerie()
 	// calls. Each errors in timeSeriesResults are returned per each mockCreateTimeSeries()
 	// call. If all errors in timeSeriesResults are used, all other mockCreateTimeSeries calls
 	// will return nil.
 	timeSeriesResults []error
-
-	errRds []errRowData
 )
 
 func init() {
@@ -58,14 +60,18 @@ func init() {
 	addToBundler = mockAddToBundler
 }
 
-func testInit() {
+// testDataInit() initializes all data needed for each test. This function must be called at the
+// beginning of each test.
+func testDataInit() {
 	bndlerProjMap = map[*bundler.Bundler]string{}
 	projRds = map[string]*[]*RowData{}
 	timeSeriesReqs = nil
 	timeSeriesResults = nil
-	errRds = nil
+	errStorage = nil
 }
 
+// Mocked functions.
+
 func mockNewMetricClient(_ context.Context, _ ...option.ClientOption) (*monitoring.MetricClient, error) {
 	return nil, nil
 }
@@ -84,6 +90,7 @@ func mockCreateTimeSeries(_ *monitoring.MetricClient, _ context.Context, req *mp
 
 func mockNewBundler(projectID string, _ interface{}, _ func(interface{})) *bundler.Bundler {
 	bndler := &bundler.Bundler{}
+	// Record the bundler's project ID.
 	bndlerProjMap[bndler] = projectID
 	return bndler
 }
@@ -92,15 +99,16 @@ func mockAddToBundler(bndler *bundler.Bundler, item interface{}, _ int) error {
 	projID := bndlerProjMap[bndler]
 	rds, ok := projRds[projID]
 	if !ok {
-		var rdsOrig []*RowData
-		rds = &rdsOrig
+		// For new project ID, create the actual slice and save its pointer.
+		var rdsSlice []*RowData
+		rds = &rdsSlice
 		projRds[projID] = rds
 	}
 	*rds = append(*rds, item.(*RowData))
 	return nil
 }
 
-// checkMetricClient checks all recorded requests in mock metric client. We only compare int64
+// checkMetricClient checks all recorded requests to the metric client. We only compare int64
 // values of the time series. To make this work, we assigned different int64 values for all valid
 // rows in the test.
 func checkMetricClient(t *testing.T, wantReqsValues [][]int64) {
@@ -136,10 +144,10 @@ type errRowData struct {
 	rds []*RowData
 }
 
-// onError records any incoming error and accompanying RowData array. This method is passed to the
-// exporter to record errors.
+// testOnError records any incoming error and accompanying RowData array. This function is passed to
+// the exporter to record errors.
 func testOnError(err error, rds ...*RowData) {
-	errRds = append(errRds, errRowData{err, rds})
+	errStorage = append(errStorage, errRowData{err, rds})
 }
 
 // errRowDataCheck contains data for checking content of error storage.
@@ -150,14 +158,14 @@ type errRowDataCheck struct {
 
 // checkErrStorage checks content of error storage. For returned errors, we check prefix and suffix.
 func checkErrStorage(t *testing.T, wantErrRdCheck []errRowDataCheck) {
-	gotLen, wantLen := len(errRds), len(wantErrRdCheck)
+	gotLen, wantLen := len(errStorage), len(wantErrRdCheck)
 	if gotLen != wantLen {
 		t.Errorf("number of reported errors: %d, want: %d", gotLen, wantLen)
 		return
 	}
 	for i := 0; i < gotLen; i++ {
 		prefix := fmt.Sprintf("%d-th reported error mismatch", i+1)
-		errRd, wantErrRd := errRds[i], wantErrRdCheck[i]
+		errRd, wantErrRd := errStorage[i], wantErrRdCheck[i]
 		errStr := errRd.err.Error()
 		if errPrefix := wantErrRd.errPrefix; !strings.HasPrefix(errStr, errPrefix) {
 			t.Errorf("%s: error got: %q, want: prefixed by %q", prefix, errStr, errPrefix)
@@ -200,7 +208,7 @@ func checkRowData(rd, wantRd *RowData) error {
 	return nil
 }
 
-// newMockExp creates mock expoter and error storage storing all errors. Caller need not set
+// newTestExp creates an exporter which saves error to errStorage. Caller should not set
 // opts.OnError.
 func newTestExp(t *testing.T, opts *Options) *Exporter {
 	opts.OnError = testOnError
@@ -211,8 +219,8 @@ func newTestExp(t *testing.T, opts *Options) *Exporter {
 	return exp
 }
 
-// checkExpProjData checks all data passed to the bundler by bundler.Add().
-func checkExpProjData(t *testing.T, wantProjData map[string][]*RowData) {
+// checkProjData checks all data passed to the bundler by bundler.Add().
+func checkProjData(t *testing.T, wantProjData map[string][]*RowData) {
 	wantProj := map[string]bool{}
 	for proj := range wantProjData {
 		wantProj[proj] = true
@@ -235,8 +243,8 @@ func checkExpProjData(t *testing.T, wantProjData map[string][]*RowData) {
 	}
 }
 
-// newMockUploader creates objects to test behavior of projectData.uploadRowData. Other uses are not
-// recommended.
+// newTestProjData creates a projectData object to test behavior of projectData.uploadRowData. Other
+// uses are not recommended. As newTestExp, all errors are saved to errStorage.
 func newTestProjData(t *testing.T, opts *Options) *projectData {
 	return newTestExp(t, opts).newProjectData(project1)
 }

monitoring/exporter/stackdriver/stackdriver_test.go

@@ -24,6 +24,7 @@ import (
 
 // This file contains actual tests.
 
+// TestAll runs all tests defined in this file.
 func TestAll(t *testing.T) {
 	testData := []struct {
 		name string
@@ -41,14 +42,14 @@ func TestAll(t *testing.T) {
 
 	for _, data := range testData {
 		run := func(t *testing.T) {
-			testInit()
+			testDataInit()
 			data.test(t)
 		}
 		t.Run(data.name, run)
 	}
 }
 
-// TestProjectClassifyNoError tests that exporter can recognize and distribute incoming data by
+// testProjectClassifyNoError tests that exporter can recognize and distribute incoming data by
 // its project.
 func testProjectClassifyNoError(t *testing.T) {
 	viewData1 := &view.Data{
@@ -89,10 +90,10 @@ func testProjectClassifyNoError(t *testing.T) {
 		},
 	}
 	checkErrStorage(t, nil)
-	checkExpProjData(t, wantRowData)
+	checkProjData(t, wantRowData)
 }
 
-// TestProjectClassifyError tests that exporter can properly handle errors while classifying
+// testProjectClassifyError tests that exporter can properly handle errors while classifying
 // incoming data by its project.
 func testProjectClassifyError(t *testing.T) {
 	viewData1 := &view.Data{
@@ -139,9 +140,11 @@ func testProjectClassifyError(t *testing.T) {
 		},
 	}
 	checkErrStorage(t, wantErrRdCheck)
-	checkExpProjData(t, wantRowData)
+	checkProjData(t, wantRowData)
 }
 
+// testDefaultProjectClassify tests that defaultGetProjectID classifies RowData by tag with key name
+// "project_id".
 func testDefaultProjectClassify(t *testing.T) {
 	viewData1 := &view.Data{
 		View:  view1,
@@ -164,10 +167,11 @@ func testDefaultProjectClassify(t *testing.T) {
 		project1: []*RowData{{view3, startTime2, endTime2, view3row1}},
 		project2: []*RowData{{view3, startTime2, endTime2, view3row2}},
 	}
-	checkExpProjData(t, wantRowData)
+	checkProjData(t, wantRowData)
 }
 
-//
+// testUploadNoError tests that all RowData objects passed to uploadRowData() are grouped by
+// slice of length MaxTimeSeriesPerUpload, and passed to createTimeSeries().
 func testUploadNoError(t *testing.T) {
 	pd := newTestProjData(t, &Options{})
 	rd := []*RowData{
@@ -187,7 +191,7 @@ func testUploadNoError(t *testing.T) {
 	checkMetricClient(t, wantClData)
 }
 
-// TestUploadTimeSeriesMakeError tests that errors while creating time series are properly handled.
+// testUploadTimeSeriesMakeError tests that errors while creating time series are properly handled.
 func testUploadTimeSeriesMakeError(t *testing.T) {
 	makeResource := func(rd *RowData) (*mrpb.MonitoredResource, error) {
 		if rd.Row == view1row2 {
@@ -227,7 +231,7 @@ func testUploadTimeSeriesMakeError(t *testing.T) {
 	checkMetricClient(t, wantClData)
 }
 
-// TestUploadTimeSeriesMakeError tests that exporter can handle error on metric client's time
+// testUploadTimeSeriesMakeError tests that exporter can handle error on metric client's time
 // series create RPC call.
 func testUploadWithMetricClientError(t *testing.T) {
 	pd := newTestProjData(t, &Options{})
@@ -261,7 +265,7 @@ func testUploadWithMetricClientError(t *testing.T) {
 	checkMetricClient(t, wantClData)
 }
 
-// TestMakeResource tests that exporter can create monitored resource dynamically.
+// testMakeResource tests that exporter can create monitored resource dynamically.
 func testMakeResource(t *testing.T) {
 	makeResource := func(rd *RowData) (*mrpb.MonitoredResource, error) {
 		switch rd.Row {
@@ -290,7 +294,7 @@ func testMakeResource(t *testing.T) {
 	}
 }
 
-// TestMakeLabel tests that exporter can correctly handle label manipulation process, including
+// testMakeLabel tests that exporter can correctly handle label manipulation process, including
 // merging default label with tags, and removing unexported labels.
 func testMakeLabel(t *testing.T) {
 	opts := &Options{