You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2021/03/03 12:43:51 UTC

[GitHub] [skywalking-infra-e2e] fgksgf opened a new pull request #10: Implement the verification of query

fgksgf opened a new pull request #10:
URL: https://github.com/apache/skywalking-infra-e2e/pull/10


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-infra-e2e] wu-sheng commented on a change in pull request #10: Implement the verification of query

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #10:
URL: https://github.com/apache/skywalking-infra-e2e/pull/10#discussion_r586413961



##########
File path: e2e.yaml
##########
@@ -0,0 +1,28 @@
+# Licensed to Apache Software Foundation (ASF) under one or more contributor
+# license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright
+# ownership. Apache Software Foundation (ASF) licenses this file to you under
+# the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# This file is used to show how to write configuration files and can be used to test.
+
+verify:

Review comment:
       `-c` should be important, it should be added before the main repo begins to adopt this.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-infra-e2e] kezhenxu94 commented on a change in pull request #10: Implement the verification of query

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #10:
URL: https://github.com/apache/skywalking-infra-e2e/pull/10#discussion_r586411188



##########
File path: e2e.yaml
##########
@@ -0,0 +1,28 @@
+# Licensed to Apache Software Foundation (ASF) under one or more contributor
+# license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright
+# ownership. Apache Software Foundation (ASF) licenses this file to you under
+# the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# This file is used to show how to write configuration files and can be used to test.
+
+verify:

Review comment:
       Let's put this in the `example` folder for now, if any of you have time, please consider adding `--config (-c)` to specify the config file path. 
   
   One thing noteworthy is that all file paths inside `e2e.yaml` should be relative paths to the `e2e.yaml`




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-infra-e2e] Humbertzhang commented on a change in pull request #10: Implement the verification of query

Posted by GitBox <gi...@apache.org>.
Humbertzhang commented on a change in pull request #10:
URL: https://github.com/apache/skywalking-infra-e2e/pull/10#discussion_r586397985



##########
File path: e2e.yaml
##########
@@ -0,0 +1,28 @@
+# Licensed to Apache Software Foundation (ASF) under one or more contributor
+# license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright
+# ownership. Apache Software Foundation (ASF) licenses this file to you under
+# the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# This file is used to show how to write configuration files and can be used to test.
+
+verify:

Review comment:
       Maybe this file should in `test` or `example` folder? What do u think?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-infra-e2e] fgksgf commented on a change in pull request #10: Implement the verification of query

Posted by GitBox <gi...@apache.org>.
fgksgf commented on a change in pull request #10:
URL: https://github.com/apache/skywalking-infra-e2e/pull/10#discussion_r586917157



##########
File path: e2e.yaml
##########
@@ -0,0 +1,28 @@
+# Licensed to Apache Software Foundation (ASF) under one or more contributor
+# license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright
+# ownership. Apache Software Foundation (ASF) licenses this file to you under
+# the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# This file is used to show how to write configuration files and can be used to test.
+
+verify:

Review comment:
       > Let's put this in the `example` folder for now, if any of you have time, please consider adding `--config (-c)` to specify the config file path.
   
   Ok, I will do it in this PR.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-infra-e2e] wu-sheng commented on a change in pull request #10: Implement the verification of query

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #10:
URL: https://github.com/apache/skywalking-infra-e2e/pull/10#discussion_r587471364



##########
File path: examples/simple/e2e.yaml
##########
@@ -32,3 +34,13 @@ setup:
           resource: pod
           for: condition=Ready
   timeout: 600
+
+verify:
+  - actual: ../../test/verify/1.actual.yaml

Review comment:
       Whether actual data is in memory or in the static yaml? Or this yaml is generated?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-infra-e2e] fgksgf commented on a change in pull request #10: Implement the verification of query

Posted by GitBox <gi...@apache.org>.
fgksgf commented on a change in pull request #10:
URL: https://github.com/apache/skywalking-infra-e2e/pull/10#discussion_r586406799



##########
File path: e2e.yaml
##########
@@ -0,0 +1,28 @@
+# Licensed to Apache Software Foundation (ASF) under one or more contributor
+# license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright
+# ownership. Apache Software Foundation (ASF) licenses this file to you under
+# the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# This file is used to show how to write configuration files and can be used to test.
+
+verify:

Review comment:
       It seems we can't specify the config file now, so I put it in the root dir for testing.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-infra-e2e] kezhenxu94 commented on a change in pull request #10: Implement the verification of query

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #10:
URL: https://github.com/apache/skywalking-infra-e2e/pull/10#discussion_r587476601



##########
File path: examples/simple/e2e.yaml
##########
@@ -32,3 +34,13 @@ setup:
           resource: pod
           for: condition=Ready
   timeout: 600
+
+verify:
+  - actual: ../../test/verify/1.actual.yaml

Review comment:
       > Whether actual data is in memory or in the static yaml? Or this yaml is generated?
   
   This example `e2e.yaml` demonstrates 2 scenarios, 1) actual data is generated in users tests, (your question 👆), 2) actual data is in-memory via query command, both are obviously not static
   
   ```yaml
   - actual: ../../test/verify/1.actual.yaml  # scenario 1
     expected: ../../test/verify/2.expected.yaml
   - query: swctl --display yaml service ls   # scenario 2
     expected: ../../test/verify/3.expected.yaml
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-infra-e2e] fgksgf commented on a change in pull request #10: Implement the verification of query

Posted by GitBox <gi...@apache.org>.
fgksgf commented on a change in pull request #10:
URL: https://github.com/apache/skywalking-infra-e2e/pull/10#discussion_r586404615



##########
File path: commands/verify/verify.go
##########
@@ -41,10 +44,62 @@ var Verify = &cobra.Command{
 	Use:   "verify",
 	Short: "verify if the actual data match the expected data",
 	RunE: func(cmd *cobra.Command, args []string) error {
-		if actual != "" && expected != "" {
-			return verifier.VerifyDataFile(actual, expected)
+		if expected != "" {
+			return verifySingleCase(expected, actual, query)
 		}
-		fmt.Println("Not implemented.")
-		return nil
+		// If there is no given flags.
+		return verifyAccordingConfig()
 	},
 }
+
+func verifySingleCase(expectedFile, actualFile, query string) error {
+	expectedData, err := util.ReadFileContent(expectedFile)
+	if err != nil {
+		return fmt.Errorf("failed to read the expected data file: %v", err)
+	}
+
+	var actualData, sourceName string
+	if actualFile != "" {
+		sourceName = actualFile
+		actualData, err = util.ReadFileContent(actualFile)
+		if err != nil {
+			return fmt.Errorf("failed to read the actual data file: %v", err)
+		}
+	} else if query != "" {
+		sourceName = query
+		actualData, err = util.ExecuteCommand(query)
+		if err != nil {
+			return fmt.Errorf("failed to execute the query: %v", err)
+		}
+	}
+
+	if err = verifier.Verify(actualData, expectedData); err != nil {
+		logger.Log.Warnf("failed to verify the output: %s\n", sourceName)
+		if me, ok := err.(*verifier.MismatchError); ok {
+			fmt.Println(me.Error())

Review comment:
       It will print the diff actually.
   https://github.com/apache/skywalking-infra-e2e/blob/10a26ec9858adfe8469e9d2ebdbd697b86c3d50d/internal/components/verifier/verifier.go#L39-L44




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-infra-e2e] kezhenxu94 commented on a change in pull request #10: Implement the verification of query

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #10:
URL: https://github.com/apache/skywalking-infra-e2e/pull/10#discussion_r586411188



##########
File path: e2e.yaml
##########
@@ -0,0 +1,28 @@
+# Licensed to Apache Software Foundation (ASF) under one or more contributor
+# license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright
+# ownership. Apache Software Foundation (ASF) licenses this file to you under
+# the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# This file is used to show how to write configuration files and can be used to test.
+
+verify:

Review comment:
       Let's put this in the `example` folder for now, if any of you have time, please consider adding `--config (-c)` to specify the config file path




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-infra-e2e] kezhenxu94 commented on a change in pull request #10: Implement the verification of query

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #10:
URL: https://github.com/apache/skywalking-infra-e2e/pull/10#discussion_r586400995



##########
File path: commands/verify/verify.go
##########
@@ -41,10 +44,62 @@ var Verify = &cobra.Command{
 	Use:   "verify",
 	Short: "verify if the actual data match the expected data",
 	RunE: func(cmd *cobra.Command, args []string) error {
-		if actual != "" && expected != "" {
-			return verifier.VerifyDataFile(actual, expected)
+		if expected != "" {
+			return verifySingleCase(expected, actual, query)
 		}
-		fmt.Println("Not implemented.")
-		return nil
+		// If there is no given flags.
+		return verifyAccordingConfig()
 	},
 }
+
+func verifySingleCase(expectedFile, actualFile, query string) error {
+	expectedData, err := util.ReadFileContent(expectedFile)
+	if err != nil {
+		return fmt.Errorf("failed to read the expected data file: %v", err)
+	}
+
+	var actualData, sourceName string
+	if actualFile != "" {
+		sourceName = actualFile
+		actualData, err = util.ReadFileContent(actualFile)
+		if err != nil {
+			return fmt.Errorf("failed to read the actual data file: %v", err)
+		}
+	} else if query != "" {
+		sourceName = query
+		actualData, err = util.ExecuteCommand(query)
+		if err != nil {
+			return fmt.Errorf("failed to execute the query: %v", err)
+		}
+	}
+
+	if err = verifier.Verify(actualData, expectedData); err != nil {
+		logger.Log.Warnf("failed to verify the output: %s\n", sourceName)
+		if me, ok := err.(*verifier.MismatchError); ok {
+			fmt.Println(me.Error())

Review comment:
       Can we use `logger.Log.Errorf` to print the error?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-infra-e2e] kezhenxu94 merged pull request #10: Implement the verification of query

Posted by GitBox <gi...@apache.org>.
kezhenxu94 merged pull request #10:
URL: https://github.com/apache/skywalking-infra-e2e/pull/10


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org