PMM-1772 Use stable instances for tests. #34

AlekSi · 2019-12-12T11:51:45Z

No description provided.

golangcibot · 2019-12-12T12:46:51Z

sessions/sessions_test.go

+	// set explicit keys to first instance to test grouping
+	cfg.Instances[0].AWSAccessKey = os.Getenv("AWS_ACCESS_KEY")
+	cfg.Instances[0].AWSSecretKey = os.Getenv("AWS_SECRET_KEY")
+	if cfg.Instances[0].AWSAccessKey == "" || cfg.Instances[0].AWSSecretKey == "" {


only one cuddle assignment allowed before if statement (from wsl)

golangcibot · 2019-12-12T12:46:51Z

sessions/sessions_test.go

+	if am56s == p10s {
+		assert.Fail(t, "autotest-aurora-mysql-56 and autotest-psql-10 should not share session - different keys (implicit and explicit)")
+	}
+	if p10s == m57s {


if statements should only be cuddled with assignments (from wsl)

golangcibot · 2019-12-12T12:46:52Z

sessions/sessions_test.go

 	}
-	if a1s != m56s {
-		assert.Fail(t, "rds-aurora1 and rds-mysql56 should share session")
+	if m57s != ap11s {


if statements should only be cuddled with assignments (from wsl)

golangcibot · 2019-12-12T12:46:52Z

sessions/sessions_test.go

 	}
-	if m57s != nil {
-		assert.Fail(t, "rds-mysql57 should be skipped")
+	if ns != nil {


if statements should only be cuddled with assignments (from wsl)

enhanced/metrics_test.go

golangcibot · 2019-12-12T14:00:25Z

enhanced/metrics.go

 	Swap              swap              `json:"swap"`
 	Tasks             tasks             `json:"tasks"`
+
+	// TODO Handle this: https://jira.percona.com/browse/PMM-3835


enhanced/metrics.go:41: Line contains TODO/BUG/FIXME: "TODO Handle this: https://jira.percona.c..." (from godox)

golangcibot · 2019-12-12T14:00:25Z

enhanced/metrics.go

 	TGID         int     `json:"tgid"         help:"The thread group identifier, which is a number representing the process ID to which a thread belongs. This identifier is used to group threads from the same process."`
 	VSS          int     `json:"vss"          help:"The amount of virtual memory allocated to the process, in kilobytes."`
+
+	// TODO Handle this.


enhanced/metrics.go:140: Line contains TODO/BUG/FIXME: "TODO Handle this." (from godox)

enhanced/scraper_test.go

golangcibot · 2019-12-12T14:00:26Z

enhanced/metrics.go

-func parseOSMetrics(b []byte) (*osMetrics, error) {
+func parseOSMetrics(b []byte, disallowUnknownFields bool) (*osMetrics, error) {
+	d := json.NewDecoder(bytes.NewReader(b))
+	if disallowUnknownFields {


if statements should only be cuddled with assignments used in the if statement itself (from wsl)

golangcibot · 2019-12-12T14:00:27Z

enhanced/metrics.go

 	for i := 0; i < t.NumField(); i++ {
 		tags := t.Field(i).Tag
 		name, help := tags.Get("json"), tags.Get("help")
+		if help == "-" {


only one cuddle assignment allowed before if statement (from wsl)

golangcibot · 2019-12-13T18:17:32Z

enhanced/helpers_test.go

+
+var (
+	golden    = flag.Bool("golden", false, "update both golden .json and .txt files")
+	goldenTXT = flag.Bool("golden-txt", false, "update golden .txt files")


var goldenTXT is unused (from unused)

golangcibot · 2019-12-13T18:17:32Z

enhanced/helpers_test.go

+	goldenTXT = flag.Bool("golden-txt", false, "update golden .txt files")
+)
+
+func readTestDataJSON(t *testing.T, instance string) []byte {


func readTestDataJSON is unused (from unused)

golangcibot · 2019-12-13T18:17:33Z

enhanced/helpers_test.go

+	require.NoError(t, err)
+}
+
+func readTestDataMetrics(t *testing.T, instance string) []string {


func readTestDataMetrics is unused (from unused)

golangcibot · 2019-12-13T18:17:33Z

enhanced/helpers_test.go

+)
+
+var (
+	golden    = flag.Bool("golden", false, "update both golden .json and .txt files")


var golden is unused (from unused)

golangcibot · 2019-12-13T18:17:33Z

enhanced/helpers_test.go

+	return bytes.TrimSpace(b)
+}
+
+func writeTestDataJSON(t *testing.T, instance string, b []byte) {


func writeTestDataJSON is unused (from unused)

golangcibot · 2019-12-13T18:17:34Z

enhanced/scraper.go

+			if allMessages[instance.ResourceID] == nil {
+				allMessages[instance.ResourceID] = make(map[time.Time]string)
+			}
+			allMessages[instance.ResourceID][timestamp] = *event.Message


assignments should only be cuddled with other assignments (from wsl)

golangcibot · 2019-12-13T18:17:34Z

enhanced/scraper.go

+		resMessages[resourceID] = allMessages[resourceID][timestamp]
 	}
-	return res
+	return resMetrics, resMessages


return statements should not be cuddled if block has more than two lines (from wsl)

golangcibot · 2019-12-13T18:17:35Z

enhanced/helpers_test.go

+
+	b, err := ioutil.ReadFile(filepath.Join("testdata", instance+".json")) //nolint:gosec
+	require.NoError(t, err)
+	return bytes.TrimSpace(b)


return statements should not be cuddled if block has more than two lines (from wsl)

golangcibot · 2019-12-13T18:17:35Z

enhanced/helpers_test.go

+
+	b, err := ioutil.ReadFile(filepath.Join("testdata", instance+".txt")) //nolint:gosec
+	require.NoError(t, err)
+	return strings.Split(string(bytes.TrimSpace(b)), "\n")


return statements should not be cuddled if block has more than two lines (from wsl)

golangcibot · 2019-12-13T18:17:35Z

enhanced/scraper_test.go

+	// if JSON was updated, update metrics too
+	if !t.Failed() && *golden {
+		*goldenTXT = true
+		TestParse(t)


only cuddled expressions if assigning variable or using from line above (from wsl)

golangcibot · 2019-12-13T18:36:34Z

basic/helpers_test.go

+	return strings.Split(string(bytes.TrimSpace(b)), "\n")
+}
+
+func writeTestDataMetrics(t *testing.T, metrics []string) {


func writeTestDataMetrics is unused (from unused)

golangcibot · 2019-12-13T18:36:35Z

sessions/sessions_test.go


+var (
+	golden    = flag.Bool("golden", false, "does nothing; exists only for compatibility with other packages")
+	goldenTXT = flag.Bool("golden-txt", false, "does nothing; exists only for compatibility with other packages")


var goldenTXT is unused (from unused)

golangcibot · 2019-12-13T18:36:35Z

basic/helpers_test.go

+
+var (
+	golden    = flag.Bool("golden", false, "does nothing; exists only for compatibility with other packages")
+	goldenTXT = flag.Bool("golden-txt", false, "update golden .txt files")


var goldenTXT is unused (from unused)

golangcibot · 2019-12-13T18:36:36Z

basic/collector_test.go

-func getCollector(t *testing.T) *Collector {
-	t.Helper()
-
+func TestCollector(t *testing.T) {


func TestCollector is unused (from unused)

golangcibot · 2019-12-13T18:36:36Z

basic/helpers_test.go

+)
+
+var (
+	golden    = flag.Bool("golden", false, "does nothing; exists only for compatibility with other packages")


var golden is unused (from unused)

golangcibot · 2019-12-13T18:36:36Z

basic/helpers_test.go

+	goldenTXT = flag.Bool("golden-txt", false, "update golden .txt files")
+)
+
+func readTestDataMetrics(t *testing.T) []string {


func readTestDataMetrics is unused (from unused)

golangcibot · 2019-12-13T18:36:37Z

sessions/sessions_test.go

 )

+var (
+	golden    = flag.Bool("golden", false, "does nothing; exists only for compatibility with other packages")


var golden is unused (from unused)

basic/collector_test.go

codecov · 2019-12-13T18:39:41Z

Codecov Report

❗ No coverage uploaded for pull request base (master@03bab8a). Click here to learn what that means.
The diff coverage is 74.28%.

@@            Coverage Diff            @@
##             master      #34   +/-   ##
=========================================
  Coverage          ?   81.59%           
=========================================
  Files             ?        9           
  Lines             ?      739           
  Branches          ?        0           
=========================================
  Hits              ?      603           
  Misses            ?      118           
  Partials          ?       18

Impacted Files	Coverage Δ
enhanced/collector.go	`0% <0%> (ø)`
enhanced/metrics.go	`94.86% <77.77%> (ø)`
enhanced/scraper.go	`76.85% <79.16%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 03bab8a...c4e189b. Read the comment docs.

basic/collector_test.go

golangcibot · 2019-12-13T18:41:14Z

basic/helpers_test.go

+
+	b, err := ioutil.ReadFile(filepath.Join("testdata", "all.txt")) //nolint:gosec
+	require.NoError(t, err)
+	return strings.Split(string(bytes.TrimSpace(b)), "\n")


return statements should not be cuddled if block has more than two lines (from wsl)

golangcibot · 2019-12-13T20:03:25Z

basic/collector_test.go

+	for _, m := range actualMetrics {
+		m.Value = 0
+	}
+	actualLines = helpers.Format(helpers.WriteMetrics(actualMetrics))


assignments should only be cuddled with other assignments (from wsl)

golangcibot · 2019-12-13T20:03:26Z

basic/collector_test.go

-		metrics = append(metrics, *helpers.ReadMetric(m))
+	expectedMetrics := helpers.ReadMetrics(helpers.Parse(readTestDataMetrics(t)))
+	sort.Slice(expectedMetrics, func(i, j int) bool { return expectedMetrics[i].Less(expectedMetrics[j]) })
+	for _, m := range expectedMetrics {


only one cuddle assignment allowed before range statement (from wsl)

golangcibot · 2019-12-13T20:03:26Z

basic/collector_test.go

+	for _, m := range expectedMetrics {
+		m.Value = 0
 	}
+	expectedLines := helpers.Format(helpers.WriteMetrics(expectedMetrics))


assignments should only be cuddled with other assignments (from wsl)

BupycHuk · 2019-12-17T09:00:02Z

enhanced/scraper.go

+			osMetrics, err := parseOSMetrics([]byte(*event.Message), s.testDisallowUnknownFields)
 			if err != nil {
+				// only for tests
+				if s.testDisallowUnknownFields {


why do we need it? How can it help with tests?

This field is set to true in tests. When there are new metrics in CloudWatch Logs, tests will fail.
See discussion at https://jira.percona.com/browse/PMM-3835 for some more background.

AlekSi · 2019-12-17T09:06:23Z

enhanced/scraper_test.go

+		t.Run(fmt.Sprint(instances), func(t *testing.T) {
+			// test that there are no new metrics
+			s := newScraper(session, instances)
+			s.testDisallowUnknownFields = true


@BupycHuk there

AlekSi added 2 commits December 12, 2019 14:48

PMM-1772 Remove old config.

5dd796d

PMM-1772 Add new config.

affb60e

AlekSi self-assigned this Dec 12, 2019

AlekSi added 3 commits December 12, 2019 14:55

PMM-1772 Update API keys.

1eb19df

PMM-1772 Update sessions tests.

03a02d9

PMM-1772 Update enhanced tests.

e5dd7ce

golangcibot reviewed Dec 12, 2019

View reviewed changes

PMM-1772 Add more tests.

a324b5c

golangcibot reviewed Dec 12, 2019

View reviewed changes

PMM-1772 More tests.

9468b6f

golangcibot reviewed Dec 13, 2019

View reviewed changes

PMM-1772 Improve basic metrics test.

6ac7a57

golangcibot reviewed Dec 13, 2019

View reviewed changes

PMM-1772 Remove config.yml for now.

727cfd8

golangcibot reviewed Dec 13, 2019

View reviewed changes

PMM-1772 Side-step go-difflib bug.

c4e189b

AlekSi marked this pull request as ready for review December 13, 2019 20:02

AlekSi requested a review from BupycHuk December 13, 2019 20:02

golangcibot reviewed Dec 13, 2019

View reviewed changes

BupycHuk approved these changes Dec 17, 2019

View reviewed changes

AlekSi commented Dec 17, 2019

View reviewed changes

AlekSi merged commit ade0b0a into master Dec 17, 2019

AlekSi deleted the PMM-1772-use-stable-instances-for-tests branch December 17, 2019 09:12

Uh oh!

PMM-1772 Use stable instances for tests. #34

PMM-1772 Use stable instances for tests. #34

Uh oh!

Conversation

AlekSi commented Dec 12, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Dec 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Dec 13, 2019 •

edited

Loading