From 876bdf9e54215452867914fec632b59757ade28e Mon Sep 17 00:00:00 2001 From: Electronic-Waste <2690692950@qq.com> Date: Tue, 5 Mar 2024 22:31:30 +0800 Subject: [PATCH 01/10] chore: add unit testcases for files in Text format. Signed-off-by: Electronic-Waste <2690692950@qq.com> --- .../file-metricscollector_test.go | 179 +++++++++++++++++- 1 file changed, 172 insertions(+), 7 deletions(-) diff --git a/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go b/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go index e9e0c99c86a..2a2a3db19c1 100644 --- a/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go +++ b/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go @@ -23,22 +23,23 @@ import ( "testing" "time" + "github.com/google/go-cmp/cmp" commonv1beta1 "github.com/kubeflow/katib/pkg/apis/controller/common/v1beta1" v1beta1 "github.com/kubeflow/katib/pkg/apis/manager/v1beta1" "github.com/kubeflow/katib/pkg/controller.v1beta1/consts" ) -var testJsonDataPath = filepath.Join("testdata", "JSON") - -func TestCollectObservationLog(t *testing.T) { +var ( + testJsonDataPath = filepath.Join("testdata", "JSON") + testTextDataPath = filepath.Join("testdata", "TEXT") +) - if err := generateTestFiles(); err != nil { +func TestCollectJsonObservationLog(t *testing.T) { + if err := generateJsonTestFiles(); err != nil { t.Fatal(err) } defer os.RemoveAll(filepath.Dir(testJsonDataPath)) - // TODO (tenzen-y): We should add tests for logs in TEXT format. - // Ref: https://github.com/kubeflow/katib/issues/1756 testCases := []struct { description string filePath string @@ -167,7 +168,131 @@ func TestCollectObservationLog(t *testing.T) { } } -func generateTestFiles() error { +func TestCollectTextObservationLog(t *testing.T) { + if err := generateTextTestFiles(); err != nil { + t.Fatal(err) + } + defer os.RemoveAll(filepath.Dir(testTextDataPath)) + + testCases := map[string]struct{ + filePath string + metrics []string + filters []string + fileFormat commonv1beta1.FileFormat + err bool + expected *v1beta1.ObservationLog + }{ + "Positive case for logs in TEXT format": { + filePath: filepath.Join(testTextDataPath, "good.log"), + metrics: []string{"accuracy", "loss"}, + filters: []string{"{metricName: ([\\w|-]+), metricValue: ((-?\\d+)(\\.\\d+)?)}"}, + fileFormat: commonv1beta1.TextFormat, + expected: &v1beta1.ObservationLog{ + MetricLogs: []*v1beta1.MetricLog{ + { + TimeStamp: "2024-03-04T17:55:08Z", + Metric: &v1beta1.Metric{ + Name: "accuracy", + Value: "0.8078", + }, + }, + { + TimeStamp: "2024-03-04T17:55:08Z", + Metric: &v1beta1.Metric{ + Name: "loss", + Value: "0.5183", + }, + }, + { + TimeStamp: "2024-03-04T17:55:08Z", + Metric: &v1beta1.Metric{ + Name: "accuracy", + Value: "0.6752", + }, + }, + { + TimeStamp: "2024-03-04T17:55:08Z", + Metric: &v1beta1.Metric{ + Name: "loss", + Value: "0.3634", + }, + }, + { + TimeStamp: time.Time{}.UTC().Format(time.RFC3339), + Metric: &v1beta1.Metric{ + Name: "loss", + Value: "0.8671", + }, + }, + }, + }, + }, + "Invalid file name": { + filePath: "invalid", + err: true, + }, + "Invalid file format": { + filePath: filepath.Join(testTextDataPath, "good.log"), + fileFormat: "invalid", + err: true, + }, + "Invalid timestamp for logs in TEXT format": { + filePath: filepath.Join(testTextDataPath, "invalid-timestamp.log"), + metrics: []string{"accuracy", "loss"}, + filters: []string{"{metricName: ([\\w|-]+), metricValue: ((-?\\d+)(\\.\\d+)?)}"}, + fileFormat: commonv1beta1.TextFormat, + expected: &v1beta1.ObservationLog{ + MetricLogs: []*v1beta1.MetricLog{ + { + TimeStamp: "2024-03-04T17:55:08Z", + Metric: &v1beta1.Metric{ + Name: "accuracy", + Value: "0.6752", + }, + }, + { + TimeStamp: time.Time{}.UTC().Format(time.RFC3339), + Metric: &v1beta1.Metric{ + Name: "loss", + Value: "0.3634", + }, + }, + }, + }, + }, + "Missing objective metric in training logs": { + filePath: filepath.Join(testTextDataPath, "missing-objective-metric.log"), + fileFormat: commonv1beta1.TextFormat, + metrics: []string{"accuracy", "loss"}, + expected: &v1beta1.ObservationLog{ + MetricLogs: []*v1beta1.MetricLog{ + { + TimeStamp: time.Time{}.UTC().Format(time.RFC3339), + Metric: &v1beta1.Metric{ + Name: "accuracy", + Value: consts.UnavailableMetricValue, + }, + }, + }, + }, + }, + } + + for name, test := range testCases { + t.Run(name, func(t *testing.T) { + actual, err := CollectObservationLog(test.filePath, test.metrics, test.filters, test.fileFormat) + if (err != nil) != test.err { + t.Errorf("\nGOT: \n%v\nWANT: %v\n", err, test.err) + } else { + if cmp.Diff(actual, test.expected) != "" { + t.Errorf("Expected %v\n got %v", test.expected, actual) + } + } + }) + } +} + +func generateJsonTestFiles() error { if _, err := os.Stat(testJsonDataPath); err != nil { if err = os.MkdirAll(testJsonDataPath, 0700); err != nil { return err @@ -215,3 +340,43 @@ func generateTestFiles() error { return nil } + +func generateTextTestFiles() error { + if _, err := os.Stat(testTextDataPath); err != nil { + if err = os.MkdirAll(testTextDataPath, 0700); err != nil { + return err + } + } + + testData := []struct { + fileName string + data string + }{ + { + fileName: "good.log", + data: `2024-03-04T17:55:08Z INFO {metricName: accuracy, metricValue: 0.8078};{metricName: loss, metricValue: 0.5183} +2024-03-04T17:55:08Z INFO {metricName: accuracy, metricValue: 0.6752} +2024-03-04T17:55:08Z INFO {metricName: loss, metricValue: 0.3634} +{metricName: loss, metricValue: 0.8671}`, + }, + { + fileName: "invalid-timestamp.log", + data: `2024-03-04T17:55:08Z INFO {metricName: accuracy, metricValue: 0.6752} +invalid INFO {metricName: loss, metricValue: 0.3634}`, + }, + { + fileName: "missing-objective-metric.log", + data: `2024-03-04T17:55:08Z INFO {metricName: loss, metricValue: 0.3634} +2024-03-04T17:55:08Z INFO {metricName: loss, metricValue: 0.8671}`, + }, + } + + for _, td := range testData { + filePath := filepath.Join(testTextDataPath, td.fileName) + if err := os.WriteFile(filePath, []byte(td.data), 0600); err != nil { + return err + } + } + + return nil +} From f6b56c463770d17cce29a170c426fc289198966d Mon Sep 17 00:00:00 2001 From: Electronic-Waste <2690692950@qq.com> Date: Wed, 6 Mar 2024 20:42:45 +0800 Subject: [PATCH 02/10] chore: adjust file layout using gofmt. Signed-off-by: Electronic-Waste <2690692950@qq.com> --- .../file-metricscollector_test.go | 46 +++++++++---------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go b/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go index 2a2a3db19c1..02591a8076d 100644 --- a/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go +++ b/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go @@ -174,19 +174,19 @@ func TestCollectTextObservationLog(t *testing.T) { } defer os.RemoveAll(filepath.Dir(testTextDataPath)) - testCases := map[string]struct{ - filePath string - metrics []string - filters []string - fileFormat commonv1beta1.FileFormat - err bool - expected *v1beta1.ObservationLog + testCases := map[string]struct { + filePath string + metrics []string + filters []string + fileFormat commonv1beta1.FileFormat + err bool + expected *v1beta1.ObservationLog }{ "Positive case for logs in TEXT format": { - filePath: filepath.Join(testTextDataPath, "good.log"), - metrics: []string{"accuracy", "loss"}, - filters: []string{"{metricName: ([\\w|-]+), metricValue: ((-?\\d+)(\\.\\d+)?)}"}, - fileFormat: commonv1beta1.TextFormat, + filePath: filepath.Join(testTextDataPath, "good.log"), + metrics: []string{"accuracy", "loss"}, + filters: []string{"{metricName: ([\\w|-]+), metricValue: ((-?\\d+)(\\.\\d+)?)}"}, + fileFormat: commonv1beta1.TextFormat, expected: &v1beta1.ObservationLog{ MetricLogs: []*v1beta1.MetricLog{ { @@ -228,19 +228,19 @@ func TestCollectTextObservationLog(t *testing.T) { }, }, "Invalid file name": { - filePath: "invalid", - err: true, + filePath: "invalid", + err: true, }, "Invalid file format": { - filePath: filepath.Join(testTextDataPath, "good.log"), - fileFormat: "invalid", - err: true, + filePath: filepath.Join(testTextDataPath, "good.log"), + fileFormat: "invalid", + err: true, }, "Invalid timestamp for logs in TEXT format": { - filePath: filepath.Join(testTextDataPath, "invalid-timestamp.log"), - metrics: []string{"accuracy", "loss"}, - filters: []string{"{metricName: ([\\w|-]+), metricValue: ((-?\\d+)(\\.\\d+)?)}"}, - fileFormat: commonv1beta1.TextFormat, + filePath: filepath.Join(testTextDataPath, "invalid-timestamp.log"), + metrics: []string{"accuracy", "loss"}, + filters: []string{"{metricName: ([\\w|-]+), metricValue: ((-?\\d+)(\\.\\d+)?)}"}, + fileFormat: commonv1beta1.TextFormat, expected: &v1beta1.ObservationLog{ MetricLogs: []*v1beta1.MetricLog{ { @@ -261,9 +261,9 @@ func TestCollectTextObservationLog(t *testing.T) { }, }, "Missing objective metric in training logs": { - filePath: filepath.Join(testTextDataPath, "missing-objective-metric.log"), - fileFormat: commonv1beta1.TextFormat, - metrics: []string{"accuracy", "loss"}, + filePath: filepath.Join(testTextDataPath, "missing-objective-metric.log"), + fileFormat: commonv1beta1.TextFormat, + metrics: []string{"accuracy", "loss"}, expected: &v1beta1.ObservationLog{ MetricLogs: []*v1beta1.MetricLog{ { From 984862b46c447154fe46bf37e64cb479930b7a5c Mon Sep 17 00:00:00 2001 From: Electronic-Waste <2690692950@qq.com> Date: Wed, 6 Mar 2024 23:29:53 +0800 Subject: [PATCH 03/10] chore: combine test for JSON and TEXT file format. Signed-off-by: Electronic-Waste <2690692950@qq.com> --- .../file-metricscollector_test.go | 192 ++++++++---------- 1 file changed, 86 insertions(+), 106 deletions(-) diff --git a/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go b/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go index 02591a8076d..1a651133863 100644 --- a/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go +++ b/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go @@ -19,7 +19,6 @@ package sidecarmetricscollector import ( "os" "path/filepath" - "reflect" "testing" "time" @@ -34,26 +33,29 @@ var ( testTextDataPath = filepath.Join("testdata", "TEXT") ) -func TestCollectJsonObservationLog(t *testing.T) { +func TestCollectObservationLog(t *testing.T) { if err := generateJsonTestFiles(); err != nil { t.Fatal(err) } defer os.RemoveAll(filepath.Dir(testJsonDataPath)) - testCases := []struct { - description string - filePath string - metrics []string - filters []string - fileFormat commonv1beta1.FileFormat - err bool - expected *v1beta1.ObservationLog + if err := generateTextTestFiles(); err != nil { + t.Fatal(err) + } + defer os.RemoveAll(filepath.Dir(testTextDataPath)) + + testCases := map[string]struct { + filePath string + metrics []string + filters []string + fileFormat commonv1beta1.FileFormat + err bool + expected *v1beta1.ObservationLog }{ - { - description: "Positive case for logs in JSON format", - filePath: filepath.Join(testJsonDataPath, "good.json"), - metrics: []string{"acc", "loss"}, - fileFormat: commonv1beta1.JsonFormat, + "Positive case for logs in JSON format": { + filePath: filepath.Join(testJsonDataPath, "good.json"), + metrics: []string{"acc", "loss"}, + fileFormat: commonv1beta1.JsonFormat, expected: &v1beta1.ObservationLog{ MetricLogs: []*v1beta1.MetricLog{ { @@ -94,94 +96,6 @@ func TestCollectJsonObservationLog(t *testing.T) { }, }, }, - { - description: "Invalid file name", - filePath: "invalid", - err: true, - }, - { - description: "Invalid file format", - filePath: filepath.Join(testJsonDataPath, "good.json"), - fileFormat: "invalid", - err: true, - }, - { - description: "Invalid formatted file for logs in JSON format", - filePath: filepath.Join(testJsonDataPath, "invalid-format.json"), - fileFormat: commonv1beta1.JsonFormat, - err: true, - }, - { - description: "Invalid timestamp for logs in JSON format", - filePath: filepath.Join(testJsonDataPath, "invalid-timestamp.json"), - fileFormat: commonv1beta1.JsonFormat, - metrics: []string{"acc", "loss"}, - expected: &v1beta1.ObservationLog{ - MetricLogs: []*v1beta1.MetricLog{ - { - TimeStamp: time.Time{}.UTC().Format(time.RFC3339), - Metric: &v1beta1.Metric{ - Name: "loss", - Value: "0.22082142531871796", - }, - }, - { - TimeStamp: "2021-12-02T05:27:27Z", - Metric: &v1beta1.Metric{ - Name: "acc", - Value: "0.9349666833877563", - }, - }, - }, - }, - }, - { - description: "Missing objective metric in training logs", - filePath: filepath.Join(testJsonDataPath, "missing-objective-metric.json"), - fileFormat: commonv1beta1.JsonFormat, - metrics: []string{"acc", "loss"}, - expected: &v1beta1.ObservationLog{ - MetricLogs: []*v1beta1.MetricLog{ - { - TimeStamp: time.Time{}.UTC().Format(time.RFC3339), - Metric: &v1beta1.Metric{ - Name: "acc", - Value: consts.UnavailableMetricValue, - }, - }, - }, - }, - }, - } - - for _, test := range testCases { - t.Run(test.description, func(t *testing.T) { - actual, err := CollectObservationLog(test.filePath, test.metrics, test.filters, test.fileFormat) - if (err != nil) != test.err { - t.Errorf("\nGOT: \n%v\nWANT: %v\n", err, test.err) - } else { - if !reflect.DeepEqual(actual, test.expected) { - t.Errorf("Expected %v\n got %v", test.expected, actual) - } - } - }) - } -} - -func TestCollectTextObservationLog(t *testing.T) { - if err := generateTextTestFiles(); err != nil { - t.Fatal(err) - } - defer os.RemoveAll(filepath.Dir(testTextDataPath)) - - testCases := map[string]struct { - filePath string - metrics []string - filters []string - fileFormat commonv1beta1.FileFormat - err bool - expected *v1beta1.ObservationLog - }{ "Positive case for logs in TEXT format": { filePath: filepath.Join(testTextDataPath, "good.log"), metrics: []string{"accuracy", "loss"}, @@ -236,6 +150,51 @@ func TestCollectTextObservationLog(t *testing.T) { fileFormat: "invalid", err: true, }, + "Invalid formatted file for logs in JSON format": { + filePath: filepath.Join(testJsonDataPath, "invalid-format.json"), + fileFormat: commonv1beta1.JsonFormat, + err: true, + }, + "Invalid formatted file for logs in TEXT format": { + filePath: filepath.Join(testTextDataPath, "invalid-format.log"), + filters: []string{"{metricName: ([\\w|-]+), metricValue: ((-?\\d+)(\\.\\d+)?)}"}, + metrics: []string{"accuracy", "loss"}, + fileFormat: commonv1beta1.TextFormat, + expected: &v1beta1.ObservationLog{ + MetricLogs: []*v1beta1.MetricLog{ + { + TimeStamp: time.Time{}.UTC().Format(time.RFC3339), + Metric: &v1beta1.Metric{ + Name: "accuracy", + Value: consts.UnavailableMetricValue, + }, + }, + }, + }, + }, + "Invalid timestamp for logs in JSON format": { + filePath: filepath.Join(testJsonDataPath, "invalid-timestamp.json"), + fileFormat: commonv1beta1.JsonFormat, + metrics: []string{"acc", "loss"}, + expected: &v1beta1.ObservationLog{ + MetricLogs: []*v1beta1.MetricLog{ + { + TimeStamp: time.Time{}.UTC().Format(time.RFC3339), + Metric: &v1beta1.Metric{ + Name: "loss", + Value: "0.22082142531871796", + }, + }, + { + TimeStamp: "2021-12-02T05:27:27Z", + Metric: &v1beta1.Metric{ + Name: "acc", + Value: "0.9349666833877563", + }, + }, + }, + }, + }, "Invalid timestamp for logs in TEXT format": { filePath: filepath.Join(testTextDataPath, "invalid-timestamp.log"), metrics: []string{"accuracy", "loss"}, @@ -260,7 +219,23 @@ func TestCollectTextObservationLog(t *testing.T) { }, }, }, - "Missing objective metric in training logs": { + "Missing objective metric in JSON training logs": { + filePath: filepath.Join(testJsonDataPath, "missing-objective-metric.json"), + fileFormat: commonv1beta1.JsonFormat, + metrics: []string{"acc", "loss"}, + expected: &v1beta1.ObservationLog{ + MetricLogs: []*v1beta1.MetricLog{ + { + TimeStamp: time.Time{}.UTC().Format(time.RFC3339), + Metric: &v1beta1.Metric{ + Name: "acc", + Value: consts.UnavailableMetricValue, + }, + }, + }, + }, + }, + "Missing objective metric in TEXT training logs": { filePath: filepath.Join(testTextDataPath, "missing-objective-metric.log"), fileFormat: commonv1beta1.TextFormat, metrics: []string{"accuracy", "loss"}, @@ -284,8 +259,8 @@ func TestCollectTextObservationLog(t *testing.T) { if (err != nil) != test.err { t.Errorf("\nGOT: \n%v\nWANT: %v\n", err, test.err) } else { - if cmp.Diff(actual, test.expected) != "" { - t.Errorf("Expected %v\n got %v", test.expected, actual) + if diff := cmp.Diff(actual, test.expected); diff != "" { + t.Errorf("Expected %v\n got %v\ndiff: %v", test.expected, actual, diff) } } }) @@ -358,6 +333,11 @@ func generateTextTestFiles() error { 2024-03-04T17:55:08Z INFO {metricName: accuracy, metricValue: 0.6752} 2024-03-04T17:55:08Z INFO {metricName: loss, metricValue: 0.3634} {metricName: loss, metricValue: 0.8671}`, + }, + { + fileName: "invalid-format.log", + data: `2024-03-04T17:55:08Z INFO {metricName: accuracy, metricValue: 0.6752 +2024-03-04T17:55:08Z INFO {metricName: loss, metricValue: 0.3634}`, }, { fileName: "invalid-timestamp.log", From 90a096ab6c066ad85867b4956117d48673893429 Mon Sep 17 00:00:00 2001 From: Electronic-Waste <2690692950@qq.com> Date: Sat, 9 Mar 2024 23:26:59 +0800 Subject: [PATCH 04/10] fix: rename file-gen functions. Signed-off-by: Electronic-Waste <2690692950@qq.com> --- .../file-metricscollector/file-metricscollector_test.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go b/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go index 1a651133863..66c8fe07464 100644 --- a/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go +++ b/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go @@ -34,12 +34,11 @@ var ( ) func TestCollectObservationLog(t *testing.T) { - if err := generateJsonTestFiles(); err != nil { + if err := generateJSONTestFiles(); err != nil { t.Fatal(err) } defer os.RemoveAll(filepath.Dir(testJsonDataPath)) - - if err := generateTextTestFiles(); err != nil { + if err := generateTEXTTestFiles(); err != nil { t.Fatal(err) } defer os.RemoveAll(filepath.Dir(testTextDataPath)) @@ -267,7 +266,7 @@ func TestCollectObservationLog(t *testing.T) { } } -func generateJsonTestFiles() error { +func generateJSONTestFiles() error { if _, err := os.Stat(testJsonDataPath); err != nil { if err = os.MkdirAll(testJsonDataPath, 0700); err != nil { return err @@ -316,7 +315,7 @@ func generateJsonTestFiles() error { return nil } -func generateTextTestFiles() error { +func generateTEXTTestFiles() error { if _, err := os.Stat(testTextDataPath); err != nil { if err = os.MkdirAll(testTextDataPath, 0700); err != nil { return err From 4dfc6593e77dbac3c93a1b634838acad069dfae9 Mon Sep 17 00:00:00 2001 From: Electronic-Waste <2690692950@qq.com> Date: Sat, 9 Mar 2024 23:30:20 +0800 Subject: [PATCH 05/10] refactor: update cmd.Diff params and log outputs. Signed-off-by: Electronic-Waste <2690692950@qq.com> --- .../file-metricscollector/file-metricscollector_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go b/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go index 66c8fe07464..79f22d2c0f7 100644 --- a/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go +++ b/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go @@ -258,8 +258,8 @@ func TestCollectObservationLog(t *testing.T) { if (err != nil) != test.err { t.Errorf("\nGOT: \n%v\nWANT: %v\n", err, test.err) } else { - if diff := cmp.Diff(actual, test.expected); diff != "" { - t.Errorf("Expected %v\n got %v\ndiff: %v", test.expected, actual, diff) + if diff := cmp.Diff(test.expected, actual); diff != "" { + t.Errorf("Unexpected parsed result (-want,+got):\n%s", diff) } } }) @@ -356,6 +356,5 @@ invalid INFO {metricName: loss, metricValue: 0.3634}`, return err } } - return nil } From 42f2d458e5437210ac77febb24cbc07fab28d761 Mon Sep 17 00:00:00 2001 From: Electronic-Waste <2690692950@qq.com> Date: Sat, 9 Mar 2024 23:57:47 +0800 Subject: [PATCH 06/10] chore: add more valid and invalid testcases for TEXT format. Signed-off-by: Electronic-Waste <2690692950@qq.com> --- .../file-metricscollector_test.go | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go b/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go index 79f22d2c0f7..990ff092b3a 100644 --- a/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go +++ b/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go @@ -130,6 +130,27 @@ func TestCollectObservationLog(t *testing.T) { Value: "0.3634", }, }, + { + TimeStamp: "2024-03-04T17:55:08Z", + Metric: &v1beta1.Metric{ + Name: "accuracy", + Value: "100", + }, + }, + { + TimeStamp: "2024-03-04T17:55:08Z", + Metric: &v1beta1.Metric{ + Name: "accuracy", + Value: "888.333", + }, + }, + { + TimeStamp: "2024-03-04T17:55:08Z", + Metric: &v1beta1.Metric{ + Name: "accuracy", + Value: "-0.4759", + }, + }, { TimeStamp: time.Time{}.UTC().Format(time.RFC3339), Metric: &v1beta1.Metric{ @@ -140,6 +161,23 @@ func TestCollectObservationLog(t *testing.T) { }, }, }, + "Invalid case for logs in TEXT format": { + filePath: filepath.Join(testTextDataPath, "invalid-value.log"), + filters: []string{"{metricName: ([\\w|-]+), metricValue: ((-?\\d+)(\\.\\d+)?)}"}, + metrics: []string{"accuracy", "loss"}, + fileFormat: commonv1beta1.TextFormat, + expected: &v1beta1.ObservationLog{ + MetricLogs: []*v1beta1.MetricLog{ + { + TimeStamp: time.Time{}.UTC().Format(time.RFC3339), + Metric: &v1beta1.Metric{ + Name: "accuracy", + Value: consts.UnavailableMetricValue, + }, + }, + }, + }, + }, "Invalid file name": { filePath: "invalid", err: true, @@ -331,7 +369,17 @@ func generateTEXTTestFiles() error { data: `2024-03-04T17:55:08Z INFO {metricName: accuracy, metricValue: 0.8078};{metricName: loss, metricValue: 0.5183} 2024-03-04T17:55:08Z INFO {metricName: accuracy, metricValue: 0.6752} 2024-03-04T17:55:08Z INFO {metricName: loss, metricValue: 0.3634} +2024-03-04T17:55:08Z INFO {metricName: accuracy, metricValue: 100} +2024-03-04T17:55:08Z INFO {metricName: accuracy, metricValue: 888.333} +2024-03-04T17:55:08Z INFO {metricName: accuracy, metricValue: -0.4759} {metricName: loss, metricValue: 0.8671}`, + }, + { + fileName: "invalid-value.log", + data: `2024-03-04T17:55:08Z INFO {metricName: accuracy, metricValue: .333} +2024-03-04T17:55:08Z INFO {metricName: accuracy, metricValue: -.333} +2024-03-04T17:55:08Z INFO {metricName: accuracy, metricValue: - 345.333} +2024-03-04T17:55:08Z INFO {metricName: accuracy, metricValue: 888.}`, }, { fileName: "invalid-format.log", From f3a2e531da2ce49423cd76ea763d11d6f439645b Mon Sep 17 00:00:00 2001 From: Electronic-Waste <2690692950@qq.com> Date: Tue, 12 Mar 2024 01:13:53 +0800 Subject: [PATCH 07/10] chore: convert testcase name to const. Signed-off-by: Electronic-Waste <2690692950@qq.com> --- .../file-metricscollector_test.go | 58 ++++++++++++------- 1 file changed, 37 insertions(+), 21 deletions(-) diff --git a/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go b/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go index 990ff092b3a..d1455a5620b 100644 --- a/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go +++ b/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go @@ -28,6 +28,22 @@ import ( "github.com/kubeflow/katib/pkg/controller.v1beta1/consts" ) +const ( + invalidFileName = "invalid" + invalidFileFormat = "invalid" + + validJSONTestFile = "good.json" + invalidFormatJSONTestFile = "invalid-format.json" + invalidTimestampJSONTestFile = "invalid-timestamp.json" + missingMetricJSONTestFile = "missing-objective-metric.json" + + validTEXTTestFile = "good.log" + invalidValueTEXTTestFile = "invalid-value.log" + invalidFormatTEXTTestFile = "invalid-format.log" + invalidTimestampTEXTTestFile = "invalid-timestamp.log" + missingMetricTEXTTestFile = "missing-objective-metric.log" +) + var ( testJsonDataPath = filepath.Join("testdata", "JSON") testTextDataPath = filepath.Join("testdata", "TEXT") @@ -52,7 +68,7 @@ func TestCollectObservationLog(t *testing.T) { expected *v1beta1.ObservationLog }{ "Positive case for logs in JSON format": { - filePath: filepath.Join(testJsonDataPath, "good.json"), + filePath: filepath.Join(testJsonDataPath, validJSONTestFile), metrics: []string{"acc", "loss"}, fileFormat: commonv1beta1.JsonFormat, expected: &v1beta1.ObservationLog{ @@ -96,7 +112,7 @@ func TestCollectObservationLog(t *testing.T) { }, }, "Positive case for logs in TEXT format": { - filePath: filepath.Join(testTextDataPath, "good.log"), + filePath: filepath.Join(testTextDataPath, validTEXTTestFile), metrics: []string{"accuracy", "loss"}, filters: []string{"{metricName: ([\\w|-]+), metricValue: ((-?\\d+)(\\.\\d+)?)}"}, fileFormat: commonv1beta1.TextFormat, @@ -162,7 +178,7 @@ func TestCollectObservationLog(t *testing.T) { }, }, "Invalid case for logs in TEXT format": { - filePath: filepath.Join(testTextDataPath, "invalid-value.log"), + filePath: filepath.Join(testTextDataPath, invalidValueTEXTTestFile), filters: []string{"{metricName: ([\\w|-]+), metricValue: ((-?\\d+)(\\.\\d+)?)}"}, metrics: []string{"accuracy", "loss"}, fileFormat: commonv1beta1.TextFormat, @@ -179,21 +195,21 @@ func TestCollectObservationLog(t *testing.T) { }, }, "Invalid file name": { - filePath: "invalid", + filePath: invalidFileName, err: true, }, "Invalid file format": { - filePath: filepath.Join(testTextDataPath, "good.log"), - fileFormat: "invalid", + filePath: filepath.Join(testTextDataPath, validTEXTTestFile), + fileFormat: invalidFileFormat, err: true, }, "Invalid formatted file for logs in JSON format": { - filePath: filepath.Join(testJsonDataPath, "invalid-format.json"), + filePath: filepath.Join(testJsonDataPath, invalidFormatJSONTestFile), fileFormat: commonv1beta1.JsonFormat, err: true, }, "Invalid formatted file for logs in TEXT format": { - filePath: filepath.Join(testTextDataPath, "invalid-format.log"), + filePath: filepath.Join(testTextDataPath, invalidFormatTEXTTestFile), filters: []string{"{metricName: ([\\w|-]+), metricValue: ((-?\\d+)(\\.\\d+)?)}"}, metrics: []string{"accuracy", "loss"}, fileFormat: commonv1beta1.TextFormat, @@ -210,7 +226,7 @@ func TestCollectObservationLog(t *testing.T) { }, }, "Invalid timestamp for logs in JSON format": { - filePath: filepath.Join(testJsonDataPath, "invalid-timestamp.json"), + filePath: filepath.Join(testJsonDataPath, invalidTimestampJSONTestFile), fileFormat: commonv1beta1.JsonFormat, metrics: []string{"acc", "loss"}, expected: &v1beta1.ObservationLog{ @@ -233,7 +249,7 @@ func TestCollectObservationLog(t *testing.T) { }, }, "Invalid timestamp for logs in TEXT format": { - filePath: filepath.Join(testTextDataPath, "invalid-timestamp.log"), + filePath: filepath.Join(testTextDataPath, invalidTimestampTEXTTestFile), metrics: []string{"accuracy", "loss"}, filters: []string{"{metricName: ([\\w|-]+), metricValue: ((-?\\d+)(\\.\\d+)?)}"}, fileFormat: commonv1beta1.TextFormat, @@ -257,7 +273,7 @@ func TestCollectObservationLog(t *testing.T) { }, }, "Missing objective metric in JSON training logs": { - filePath: filepath.Join(testJsonDataPath, "missing-objective-metric.json"), + filePath: filepath.Join(testJsonDataPath, missingMetricJSONTestFile), fileFormat: commonv1beta1.JsonFormat, metrics: []string{"acc", "loss"}, expected: &v1beta1.ObservationLog{ @@ -273,7 +289,7 @@ func TestCollectObservationLog(t *testing.T) { }, }, "Missing objective metric in TEXT training logs": { - filePath: filepath.Join(testTextDataPath, "missing-objective-metric.log"), + filePath: filepath.Join(testTextDataPath, missingMetricTEXTTestFile), fileFormat: commonv1beta1.TextFormat, metrics: []string{"accuracy", "loss"}, expected: &v1beta1.ObservationLog{ @@ -316,7 +332,7 @@ func generateJSONTestFiles() error { data string }{ { - fileName: "good.json", + fileName: validJSONTestFile, data: `{"checkpoint_path": "", "global_step": "0", "loss": "0.22082142531871796", "timestamp": 1638422847.28721, "trial": "0"} {"acc": "0.9349666833877563", "checkpoint_path": "", "global_step": "0", "timestamp": 1638422847.287801, "trial": "0"} {"checkpoint_path": "", "global_step": "1", "loss": "0.1414974331855774", "timestamp": "2021-12-02T14:27:50.000035161Z", "trial": "0"} @@ -325,18 +341,18 @@ func generateJSONTestFiles() error { `, }, { - fileName: "invalid-format.json", + fileName: invalidFormatJSONTestFile, data: `"checkpoint_path": "", "global_step": "0", "loss": "0.22082142531871796", "timestamp": 1638422847.28721, "trial": "0" {"acc": "0.9349666833877563", "checkpoint_path": "", "global_step": "0", "timestamp": 1638422847.287801, "trial": "0 `, }, { - fileName: "invalid-timestamp.json", + fileName: invalidTimestampJSONTestFile, data: `{"checkpoint_path": "", "global_step": "0", "loss": "0.22082142531871796", "timestamp": "invalid", "trial": "0"} {"acc": "0.9349666833877563", "checkpoint_path": "", "global_step": "0", "timestamp": 1638422847, "trial": "0"} `, }, { - fileName: "missing-objective-metric.json", + fileName: missingMetricJSONTestFile, data: `{"checkpoint_path": "", "global_step": "0", "loss": "0.22082142531871796", "timestamp": 1638422847.28721, "trial": "0"} {"checkpoint_path": "", "global_step": "1", "loss": "0.1414974331855774", "timestamp": "2021-12-02T14:27:50.000035161+09:00", "trial": "0"} {"checkpoint_path": "", "global_step": "2", "loss": "0.10683439671993256", "trial": "0"}`, @@ -365,7 +381,7 @@ func generateTEXTTestFiles() error { data string }{ { - fileName: "good.log", + fileName: validTEXTTestFile, data: `2024-03-04T17:55:08Z INFO {metricName: accuracy, metricValue: 0.8078};{metricName: loss, metricValue: 0.5183} 2024-03-04T17:55:08Z INFO {metricName: accuracy, metricValue: 0.6752} 2024-03-04T17:55:08Z INFO {metricName: loss, metricValue: 0.3634} @@ -375,24 +391,24 @@ func generateTEXTTestFiles() error { {metricName: loss, metricValue: 0.8671}`, }, { - fileName: "invalid-value.log", + fileName: invalidValueTEXTTestFile, data: `2024-03-04T17:55:08Z INFO {metricName: accuracy, metricValue: .333} 2024-03-04T17:55:08Z INFO {metricName: accuracy, metricValue: -.333} 2024-03-04T17:55:08Z INFO {metricName: accuracy, metricValue: - 345.333} 2024-03-04T17:55:08Z INFO {metricName: accuracy, metricValue: 888.}`, }, { - fileName: "invalid-format.log", + fileName: invalidFormatTEXTTestFile, data: `2024-03-04T17:55:08Z INFO {metricName: accuracy, metricValue: 0.6752 2024-03-04T17:55:08Z INFO {metricName: loss, metricValue: 0.3634}`, }, { - fileName: "invalid-timestamp.log", + fileName: invalidTimestampTEXTTestFile, data: `2024-03-04T17:55:08Z INFO {metricName: accuracy, metricValue: 0.6752} invalid INFO {metricName: loss, metricValue: 0.3634}`, }, { - fileName: "missing-objective-metric.log", + fileName: missingMetricTEXTTestFile, data: `2024-03-04T17:55:08Z INFO {metricName: loss, metricValue: 0.3634} 2024-03-04T17:55:08Z INFO {metricName: loss, metricValue: 0.8671}`, }, From c5554f14195e6b9ace66cabe92a019263caf29eb Mon Sep 17 00:00:00 2001 From: Electronic-Waste <2690692950@qq.com> Date: Tue, 12 Mar 2024 08:58:59 +0800 Subject: [PATCH 08/10] chore: compact dir generation & deletion operations into funcs. Signed-off-by: Electronic-Waste <2690692950@qq.com> --- .../file-metricscollector_test.go | 39 ++++++++++++++----- 1 file changed, 30 insertions(+), 9 deletions(-) diff --git a/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go b/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go index d1455a5620b..799e9984f56 100644 --- a/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go +++ b/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go @@ -50,14 +50,16 @@ var ( ) func TestCollectObservationLog(t *testing.T) { + if err := generateTestDirs(); err != nil { + t.Fatal(err) + } if err := generateJSONTestFiles(); err != nil { t.Fatal(err) } - defer os.RemoveAll(filepath.Dir(testJsonDataPath)) if err := generateTEXTTestFiles(); err != nil { t.Fatal(err) } - defer os.RemoveAll(filepath.Dir(testTextDataPath)) + defer deleteTestDirs() testCases := map[string]struct { filePath string @@ -320,13 +322,37 @@ func TestCollectObservationLog(t *testing.T) { } } -func generateJSONTestFiles() error { +func generateTestDirs() error { + // Generate JSON files' dir if _, err := os.Stat(testJsonDataPath); err != nil { if err = os.MkdirAll(testJsonDataPath, 0700); err != nil { return err } } + // Generate TEXT files' dir + if _, err := os.Stat(testTextDataPath); err != nil { + if err = os.MkdirAll(testTextDataPath, 0700); err != nil { + return err + } + } + + return nil +} + +func deleteTestDirs() error { + if err := os.RemoveAll(filepath.Dir(testJsonDataPath)); err != nil { + return err + } + + if err := os.RemoveAll(filepath.Dir(testTextDataPath)); err != nil { + return err + } + + return nil +} + +func generateJSONTestFiles() error { testData := []struct { fileName string data string @@ -370,12 +396,6 @@ func generateJSONTestFiles() error { } func generateTEXTTestFiles() error { - if _, err := os.Stat(testTextDataPath); err != nil { - if err = os.MkdirAll(testTextDataPath, 0700); err != nil { - return err - } - } - testData := []struct { fileName string data string @@ -420,5 +440,6 @@ invalid INFO {metricName: loss, metricValue: 0.3634}`, return err } } + return nil } From 2496e94281f3c613b8b493cfb8a4baedbd8ec73d Mon Sep 17 00:00:00 2001 From: Electronic-Waste <2690692950@qq.com> Date: Tue, 12 Mar 2024 20:45:42 +0800 Subject: [PATCH 09/10] fix: delete constants used only once. Signed-off-by: Electronic-Waste <2690692950@qq.com> --- .../file-metricscollector/file-metricscollector_test.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go b/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go index 799e9984f56..0a0fa09bc9d 100644 --- a/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go +++ b/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go @@ -29,9 +29,6 @@ import ( ) const ( - invalidFileName = "invalid" - invalidFileFormat = "invalid" - validJSONTestFile = "good.json" invalidFormatJSONTestFile = "invalid-format.json" invalidTimestampJSONTestFile = "invalid-timestamp.json" @@ -197,12 +194,12 @@ func TestCollectObservationLog(t *testing.T) { }, }, "Invalid file name": { - filePath: invalidFileName, + filePath: "invalid", err: true, }, "Invalid file format": { filePath: filepath.Join(testTextDataPath, validTEXTTestFile), - fileFormat: invalidFileFormat, + fileFormat: "invalid", err: true, }, "Invalid formatted file for logs in JSON format": { From 82c248f89f0768487f76f88b5cf5e5b27f70f8c7 Mon Sep 17 00:00:00 2001 From: Electronic-Waste <2690692950@qq.com> Date: Tue, 12 Mar 2024 21:20:11 +0800 Subject: [PATCH 10/10] fix: fix ci error in errorcheck. Signed-off-by: Electronic-Waste <2690692950@qq.com> --- .../file-metricscollector_test.go | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go b/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go index 0a0fa09bc9d..c286bce6224 100644 --- a/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go +++ b/pkg/metricscollector/v1beta1/file-metricscollector/file-metricscollector_test.go @@ -42,8 +42,9 @@ const ( ) var ( - testJsonDataPath = filepath.Join("testdata", "JSON") - testTextDataPath = filepath.Join("testdata", "TEXT") + testDir = "testdata" + testJsonDataPath = filepath.Join(testDir, "JSON") + testTextDataPath = filepath.Join(testDir, "TEXT") ) func TestCollectObservationLog(t *testing.T) { @@ -56,7 +57,7 @@ func TestCollectObservationLog(t *testing.T) { if err := generateTEXTTestFiles(); err != nil { t.Fatal(err) } - defer deleteTestDirs() + defer os.RemoveAll(testDir) testCases := map[string]struct { filePath string @@ -337,18 +338,6 @@ func generateTestDirs() error { return nil } -func deleteTestDirs() error { - if err := os.RemoveAll(filepath.Dir(testJsonDataPath)); err != nil { - return err - } - - if err := os.RemoveAll(filepath.Dir(testTextDataPath)); err != nil { - return err - } - - return nil -} - func generateJSONTestFiles() error { testData := []struct { fileName string