You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@amoro.apache.org by "lianneli (via GitHub)" <gi...@apache.org> on 2024/04/28 09:54:45 UTC

[PR] [AMORO-2778][Helm Chart] Add Tests for Helm Chart [amoro]

lianneli opened a new pull request, #2794:
URL: https://github.com/apache/amoro/pull/2794

   ## Why are the changes needed?
   Fix #2778 
   Close #2778 
   
   ## Brief change log
   - Add Tests for templates in charts/amoro/templates
   - Add unit test usage description  to README file
   - Fix file name spelling error: "charts/amoro/templates/amoro-database-secert.yaml" to "charts/amoro/templates/amoro-database-secret.yaml"
   - Fix to delete repeated Annotation `app.kubernetes.io/name: amoro-database-secret` in "charts/amoro/templates/amoro-database-secert.yaml"
   
   ## How was this patch tested?
   
   - [x] Add some test cases that check the changes thoroughly including negative and positive cases if possible
   
   - [x] Add screenshots for manual tests if appropriate
   
   - [x] Run test locally before making a pull request
   
   ![image](https://github.com/apache/amoro/assets/19421187/934267b2-81f9-4816-b84c-3f08456eca80)
   
   
   ## Documentation
   
   - Does this pull request introduce a new feature? yes
   - If yes, how is the feature documented?  docs(README.md)


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

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


Re: [PR] [AMORO-2778][Helm Chart] Add Tests for Helm Chart [amoro]

Posted by "baiyangtx (via GitHub)" <gi...@apache.org>.
baiyangtx merged PR #2794:
URL: https://github.com/apache/amoro/pull/2794


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

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


Re: [PR] [AMORO-2778][Helm Chart] Add Tests for Helm Chart [amoro]

Posted by "lianneli (via GitHub)" <gi...@apache.org>.
lianneli commented on PR #2794:
URL: https://github.com/apache/amoro/pull/2794#issuecomment-2093214437

   Thanks again. I have accepted all the suggestions. Is there another one?
   
   @baiyangtx 


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

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


Re: [PR] [AMORO-2778][Helm Chart] Add Tests for Helm Chart [amoro]

Posted by "tcodehuber (via GitHub)" <gi...@apache.org>.
tcodehuber commented on code in PR #2794:
URL: https://github.com/apache/amoro/pull/2794#discussion_r1584223428


##########
charts/amoro/README.md:
##########
@@ -62,4 +62,20 @@ Configuration guide documentation for Amoro lives [on the website](https://amoro
 
 ## Contributing
 
-Want to help build Amoro? Check out our [contributing documentation](https://amoro.apache.org/join-community/).
\ No newline at end of file
+Want to help build Amoro? Check out our [contributing documentation](https://amoro.apache.org/join-community/).
+
+### Unit Test
+
+Helm Chart for Amoro supports Unit Tests by [helm-unittest](https://github.com/helm-unittest/helm-unittest). Before 
+submitting PRs, please let all the tests passed .

Review Comment:
   please let all tests passed.



##########
charts/amoro/tests/amoro-ingress_test.yaml:
##########
@@ -0,0 +1,222 @@
+#  Licensed to the 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.  The 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.
+
+
+suite: Test Amoro Ingress
+templates:
+  - amoro-ingress.yaml
+chart:
+  appVersion: 0.1.0
+  version: 0.6.0
+tests:
+  - it: Amoro Ingress should not exist when enabled to false
+    set:
+      ingress:
+        enabled: false
+    asserts:
+      - hasDocuments:
+          count: 0
+  - it: Amoro Ingress should exist when enabled to true
+    set:
+      fullnameOverride: test
+      ingress:
+        enabled: true
+    asserts:
+      - equal:
+          path: metadata.name
+          value: test
+      - hasDocuments:
+          count: 1
+  - it: Amoro Ingress should contains common labels and common annotations if set
+    set:
+      ingress:
+        enabled: true
+      commonLabels:
+        LABEL: LABEL_V
+      commonAnnotations:
+        ANNOTATION: ANNOTATION_V
+    asserts:
+      - exists:
+          path: metadata.labels
+      - isSubset:
+          path: metadata.labels
+          content:
+            LABEL: LABEL_V
+      - exists:
+          path: metadata.annotations
+      - isSubset:
+          path: metadata.annotations
+          content:
+            ANNOTATION: ANNOTATION_V
+  - it: Amoro Ingress should contains ingress annotations if set
+    set:
+      ingress:
+        enabled: true
+        annotations:
+          ANNOTATION: ANNOTATION_V2
+    asserts:
+      - exists:
+          path: metadata.annotations
+      - isSubset:
+          path: metadata.annotations
+          content:
+            ANNOTATION: ANNOTATION_V2
+  - it: Amoro Ingress should not contains annotations when annotations not set
+    set:
+      ingress:
+        enabled: true
+    asserts:
+      - notExists:
+          path: metadata.annotations
+  - it: Amoro Ingress should not contains ingress className if not set

Review Comment:
   Amoro Ingress should not contain ingress className if not set.



##########
charts/amoro/tests/amoro-database-secret_test.yaml:
##########
@@ -0,0 +1,86 @@
+#  Licensed to the 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.  The 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.
+
+
+suite: Test Amoro Database Secret
+templates:
+  - amoro-database-secret.yaml
+chart:
+  appVersion: 0.1.0
+  version: 0.6.0
+tests:
+  - it: Amoro database secret should not exist when database type set to derby
+    set:
+      amoroConf:
+        database:
+          type: derby
+    asserts:
+      - hasDocuments:
+          count: 0
+  - it: Amoro database secret should exist when database type set to mysql

Review Comment:
   when database type is set to mysql



##########
charts/amoro/tests/amoro-database-secret_test.yaml:
##########
@@ -0,0 +1,86 @@
+#  Licensed to the 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.  The 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.
+
+
+suite: Test Amoro Database Secret
+templates:
+  - amoro-database-secret.yaml
+chart:
+  appVersion: 0.1.0
+  version: 0.6.0
+tests:
+  - it: Amoro database secret should not exist when database type set to derby

Review Comment:
   when database type is set to derby



##########
charts/amoro/tests/amoro-database-secret_test.yaml:
##########
@@ -0,0 +1,86 @@
+#  Licensed to the 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.  The 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.
+
+
+suite: Test Amoro Database Secret
+templates:
+  - amoro-database-secret.yaml
+chart:
+  appVersion: 0.1.0
+  version: 0.6.0
+tests:
+  - it: Amoro database secret should not exist when database type set to derby
+    set:
+      amoroConf:
+        database:
+          type: derby
+    asserts:
+      - hasDocuments:
+          count: 0
+  - it: Amoro database secret should exist when database type set to mysql

Review Comment:
   Some similar contents in this file can be changed



##########
charts/amoro/tests/amoro-ingress_test.yaml:
##########
@@ -0,0 +1,222 @@
+#  Licensed to the 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.  The 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.
+
+
+suite: Test Amoro Ingress
+templates:
+  - amoro-ingress.yaml
+chart:
+  appVersion: 0.1.0
+  version: 0.6.0
+tests:
+  - it: Amoro Ingress should not exist when enabled to false
+    set:
+      ingress:
+        enabled: false
+    asserts:
+      - hasDocuments:
+          count: 0
+  - it: Amoro Ingress should exist when enabled to true
+    set:
+      fullnameOverride: test
+      ingress:
+        enabled: true
+    asserts:
+      - equal:
+          path: metadata.name
+          value: test
+      - hasDocuments:
+          count: 1
+  - it: Amoro Ingress should contains common labels and common annotations if set
+    set:
+      ingress:
+        enabled: true
+      commonLabels:
+        LABEL: LABEL_V
+      commonAnnotations:
+        ANNOTATION: ANNOTATION_V
+    asserts:
+      - exists:
+          path: metadata.labels
+      - isSubset:
+          path: metadata.labels
+          content:
+            LABEL: LABEL_V
+      - exists:
+          path: metadata.annotations
+      - isSubset:
+          path: metadata.annotations
+          content:
+            ANNOTATION: ANNOTATION_V
+  - it: Amoro Ingress should contains ingress annotations if set
+    set:
+      ingress:
+        enabled: true
+        annotations:
+          ANNOTATION: ANNOTATION_V2
+    asserts:
+      - exists:
+          path: metadata.annotations
+      - isSubset:
+          path: metadata.annotations
+          content:
+            ANNOTATION: ANNOTATION_V2
+  - it: Amoro Ingress should not contains annotations when annotations not set
+    set:
+      ingress:
+        enabled: true
+    asserts:
+      - notExists:
+          path: metadata.annotations
+  - it: Amoro Ingress should not contains ingress className if not set

Review Comment:
   contains -> contain



##########
charts/amoro/tests/amoro-service_test.yaml:
##########
@@ -0,0 +1,265 @@
+#  Licensed to the 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.  The 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.
+
+
+suite: Test Amoro Service
+templates:
+  - amoro-service.yaml
+chart:
+  appVersion: 0.1.0
+  version: 0.6.0
+tests:
+  - it: Amoro Service should contains 3 service by default value
+    asserts:
+      - hasDocuments:
+          count: 3
+  - it: Amoro Service should count 2 when rest service enabled to false
+    set:
+      server:
+        rest:
+          enabled: false
+    asserts:
+      - hasDocuments:
+          count: 2
+  - it: Amoro Service should count 1 when rest and table service enabled to false
+    set:
+      server:
+        rest:
+          enabled: false
+        table:
+          enabled: false
+    asserts:
+      - hasDocuments:
+          count: 1
+  - it: Amoro Service should count 0 when all the service enabled to false
+    set:
+      server:
+        rest:
+          enabled: false
+        table:
+          enabled: false
+        optimizing:
+          enabled: false
+    asserts:
+      - hasDocuments:
+          count: 0
+  - it: Amoro Service for rest should contains service annotations if set
+    set:
+      fullnameOverride: test
+      server:
+        rest:
+          service:
+            annotations:
+              ANNOTATION: ANNOTATION_V2
+    asserts:
+      - exists:
+          path: metadata.annotations
+        documentSelector:
+          path: metadata.name
+          value: test-rest
+      - isSubset:
+          path: metadata.annotations
+          content:
+            ANNOTATION: ANNOTATION_V2
+        documentSelector:
+          path: metadata.name
+          value: test-rest
+  - it: Amoro Service for table should contains service annotations if set
+    set:
+      fullnameOverride: test
+      server:
+        table:
+          service:
+            annotations:
+              ANNOTATION: ANNOTATION_V3
+    asserts:
+      - exists:
+          path: metadata.annotations
+        documentSelector:
+          path: metadata.name
+          value: test-table
+      - isSubset:
+          path: metadata.annotations
+          content:
+            ANNOTATION: ANNOTATION_V3
+        documentSelector:
+          path: metadata.name
+          value: test-table
+  - it: Amoro Service for optimizing should contains service annotations if set
+    set:
+      fullnameOverride: test
+      server:
+        optimizing:
+          service:
+            annotations:
+              ANNOTATION: ANNOTATION_V4
+    asserts:
+      - exists:
+          path: metadata.annotations
+        documentSelector:
+          path: metadata.name
+          value: test-optimizing
+      - isSubset:
+          path: metadata.annotations
+          content:
+            ANNOTATION: ANNOTATION_V4
+        documentSelector:
+          path: metadata.name
+          value: test-optimizing
+  - it: Amoro Service for rest should reflect ports
+    set:
+      fullnameOverride: test
+      server:
+        rest:
+          service:
+            port: "1234"
+    asserts:
+      - equal:
+          path: spec.type
+          value: ClusterIP
+        documentSelector:
+          path: metadata.name
+          value: test-rest
+      - contains:
+          path: spec.ports
+          content:
+            name: rest
+            port: 1234
+            targetPort: 1630
+        documentSelector:
+          path: metadata.name
+          value: test-rest
+  - it: Amoro Service for rest should reflect Node port of set type to NodePort
+    set:
+      fullnameOverride: test
+      server:
+        rest:
+          service:
+            type: "NodePort"
+            nodePort: "1234"
+    asserts:
+      - equal:
+          path: spec.type
+          value: NodePort
+        documentSelector:
+          path: metadata.name
+          value: test-rest
+      - contains:
+          path: spec.ports
+          content:
+            name: rest
+            nodePort: 1234
+            port: 1630
+            targetPort: 1630
+        documentSelector:
+          path: metadata.name
+          value: test-rest
+  - it: Amoro Service for table should reflect ports
+    set:
+      fullnameOverride: test
+      server:
+        table:
+          service:
+            port: "1234"
+    asserts:
+      - equal:
+          path: spec.type
+          value: ClusterIP
+        documentSelector:
+          path: metadata.name
+          value: test-table
+      - contains:
+          path: spec.ports
+          content:
+            name: table
+            port: 1234
+            targetPort: 1260
+        documentSelector:
+          path: metadata.name
+          value: test-table
+  - it: Amoro Service for table should reflect Node port of set type to NodePort

Review Comment:
   type is set to NodePort



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

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


Re: [PR] [AMORO-2778][Helm Chart] Add Tests for Helm Chart [amoro]

Posted by "lianneli (via GitHub)" <gi...@apache.org>.
lianneli commented on PR #2794:
URL: https://github.com/apache/amoro/pull/2794#issuecomment-2081417273

   @baiyangtx Could you please help to review?


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

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


Re: [PR] [AMORO-2778][Helm Chart] Add Tests for Helm Chart [amoro]

Posted by "lianneli (via GitHub)" <gi...@apache.org>.
lianneli commented on code in PR #2794:
URL: https://github.com/apache/amoro/pull/2794#discussion_r1584350099


##########
charts/amoro/README.md:
##########
@@ -62,4 +62,20 @@ Configuration guide documentation for Amoro lives [on the website](https://amoro
 
 ## Contributing
 
-Want to help build Amoro? Check out our [contributing documentation](https://amoro.apache.org/join-community/).
\ No newline at end of file
+Want to help build Amoro? Check out our [contributing documentation](https://amoro.apache.org/join-community/).
+
+### Unit Test
+
+Helm Chart for Amoro supports Unit Tests by [helm-unittest](https://github.com/helm-unittest/helm-unittest). Before 
+submitting PRs, please let all the tests passed .

Review Comment:
   fixed.



##########
charts/amoro/tests/amoro-database-secret_test.yaml:
##########
@@ -0,0 +1,86 @@
+#  Licensed to the 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.  The 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.
+
+
+suite: Test Amoro Database Secret
+templates:
+  - amoro-database-secret.yaml
+chart:
+  appVersion: 0.1.0
+  version: 0.6.0
+tests:
+  - it: Amoro database secret should not exist when database type set to derby

Review Comment:
   fixed.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

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


Re: [PR] [AMORO-2778][Helm Chart] Add Tests for Helm Chart [amoro]

Posted by "baiyangtx (via GitHub)" <gi...@apache.org>.
baiyangtx commented on code in PR #2794:
URL: https://github.com/apache/amoro/pull/2794#discussion_r1585844244


##########
.github/workflows/pull-request-on-helmchat.yml:
##########
@@ -0,0 +1,56 @@
+#
+# Licensed to the 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.
+# The 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 workflow will execute Tests when submit a PR.
+
+name: "Pull Request Test For Helm Charts"

Review Comment:
   ```suggestion
   name: "CI for Helm Charts"
   ```
   
   And the file name could be `helm-charts-ci.yml`. 



##########
.github/workflows/pull-request-on-helmchat.yml:
##########
@@ -0,0 +1,56 @@
+#
+# Licensed to the 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.
+# The 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 workflow will execute Tests when submit a PR.
+
+name: "Pull Request Test For Helm Charts"
+

Review Comment:
   Add this 
   
   ```
   concurrency:
     group: ${{ github.workflow }}-${{ github.ref }}
     cancel-in-progress: true
   ```



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

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


Re: [PR] [AMORO-2778][Helm Chart] Add Tests for Helm Chart [amoro]

Posted by "lianneli (via GitHub)" <gi...@apache.org>.
lianneli commented on code in PR #2794:
URL: https://github.com/apache/amoro/pull/2794#discussion_r1584351155


##########
charts/amoro/tests/amoro-service_test.yaml:
##########
@@ -0,0 +1,265 @@
+#  Licensed to the 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.  The 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.
+
+
+suite: Test Amoro Service
+templates:
+  - amoro-service.yaml
+chart:
+  appVersion: 0.1.0
+  version: 0.6.0
+tests:
+  - it: Amoro Service should contains 3 service by default value
+    asserts:
+      - hasDocuments:
+          count: 3
+  - it: Amoro Service should count 2 when rest service enabled to false
+    set:
+      server:
+        rest:
+          enabled: false
+    asserts:
+      - hasDocuments:
+          count: 2
+  - it: Amoro Service should count 1 when rest and table service enabled to false
+    set:
+      server:
+        rest:
+          enabled: false
+        table:
+          enabled: false
+    asserts:
+      - hasDocuments:
+          count: 1
+  - it: Amoro Service should count 0 when all the service enabled to false
+    set:
+      server:
+        rest:
+          enabled: false
+        table:
+          enabled: false
+        optimizing:
+          enabled: false
+    asserts:
+      - hasDocuments:
+          count: 0
+  - it: Amoro Service for rest should contains service annotations if set
+    set:
+      fullnameOverride: test
+      server:
+        rest:
+          service:
+            annotations:
+              ANNOTATION: ANNOTATION_V2
+    asserts:
+      - exists:
+          path: metadata.annotations
+        documentSelector:
+          path: metadata.name
+          value: test-rest
+      - isSubset:
+          path: metadata.annotations
+          content:
+            ANNOTATION: ANNOTATION_V2
+        documentSelector:
+          path: metadata.name
+          value: test-rest
+  - it: Amoro Service for table should contains service annotations if set
+    set:
+      fullnameOverride: test
+      server:
+        table:
+          service:
+            annotations:
+              ANNOTATION: ANNOTATION_V3
+    asserts:
+      - exists:
+          path: metadata.annotations
+        documentSelector:
+          path: metadata.name
+          value: test-table
+      - isSubset:
+          path: metadata.annotations
+          content:
+            ANNOTATION: ANNOTATION_V3
+        documentSelector:
+          path: metadata.name
+          value: test-table
+  - it: Amoro Service for optimizing should contains service annotations if set
+    set:
+      fullnameOverride: test
+      server:
+        optimizing:
+          service:
+            annotations:
+              ANNOTATION: ANNOTATION_V4
+    asserts:
+      - exists:
+          path: metadata.annotations
+        documentSelector:
+          path: metadata.name
+          value: test-optimizing
+      - isSubset:
+          path: metadata.annotations
+          content:
+            ANNOTATION: ANNOTATION_V4
+        documentSelector:
+          path: metadata.name
+          value: test-optimizing
+  - it: Amoro Service for rest should reflect ports
+    set:
+      fullnameOverride: test
+      server:
+        rest:
+          service:
+            port: "1234"
+    asserts:
+      - equal:
+          path: spec.type
+          value: ClusterIP
+        documentSelector:
+          path: metadata.name
+          value: test-rest
+      - contains:
+          path: spec.ports
+          content:
+            name: rest
+            port: 1234
+            targetPort: 1630
+        documentSelector:
+          path: metadata.name
+          value: test-rest
+  - it: Amoro Service for rest should reflect Node port of set type to NodePort
+    set:
+      fullnameOverride: test
+      server:
+        rest:
+          service:
+            type: "NodePort"
+            nodePort: "1234"
+    asserts:
+      - equal:
+          path: spec.type
+          value: NodePort
+        documentSelector:
+          path: metadata.name
+          value: test-rest
+      - contains:
+          path: spec.ports
+          content:
+            name: rest
+            nodePort: 1234
+            port: 1630
+            targetPort: 1630
+        documentSelector:
+          path: metadata.name
+          value: test-rest
+  - it: Amoro Service for table should reflect ports
+    set:
+      fullnameOverride: test
+      server:
+        table:
+          service:
+            port: "1234"
+    asserts:
+      - equal:
+          path: spec.type
+          value: ClusterIP
+        documentSelector:
+          path: metadata.name
+          value: test-table
+      - contains:
+          path: spec.ports
+          content:
+            name: table
+            port: 1234
+            targetPort: 1260
+        documentSelector:
+          path: metadata.name
+          value: test-table
+  - it: Amoro Service for table should reflect Node port of set type to NodePort

Review Comment:
   fixed.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

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


Re: [PR] [AMORO-2778][Helm Chart] Add Tests for Helm Chart [amoro]

Posted by "lianneli (via GitHub)" <gi...@apache.org>.
lianneli commented on code in PR #2794:
URL: https://github.com/apache/amoro/pull/2794#discussion_r1584350624


##########
charts/amoro/tests/amoro-database-secret_test.yaml:
##########
@@ -0,0 +1,86 @@
+#  Licensed to the 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.  The 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.
+
+
+suite: Test Amoro Database Secret
+templates:
+  - amoro-database-secret.yaml
+chart:
+  appVersion: 0.1.0
+  version: 0.6.0
+tests:
+  - it: Amoro database secret should not exist when database type set to derby
+    set:
+      amoroConf:
+        database:
+          type: derby
+    asserts:
+      - hasDocuments:
+          count: 0
+  - it: Amoro database secret should exist when database type set to mysql

Review Comment:
   fixed.



##########
charts/amoro/tests/amoro-ingress_test.yaml:
##########
@@ -0,0 +1,222 @@
+#  Licensed to the 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.  The 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.
+
+
+suite: Test Amoro Ingress
+templates:
+  - amoro-ingress.yaml
+chart:
+  appVersion: 0.1.0
+  version: 0.6.0
+tests:
+  - it: Amoro Ingress should not exist when enabled to false
+    set:
+      ingress:
+        enabled: false
+    asserts:
+      - hasDocuments:
+          count: 0
+  - it: Amoro Ingress should exist when enabled to true
+    set:
+      fullnameOverride: test
+      ingress:
+        enabled: true
+    asserts:
+      - equal:
+          path: metadata.name
+          value: test
+      - hasDocuments:
+          count: 1
+  - it: Amoro Ingress should contains common labels and common annotations if set
+    set:
+      ingress:
+        enabled: true
+      commonLabels:
+        LABEL: LABEL_V
+      commonAnnotations:
+        ANNOTATION: ANNOTATION_V
+    asserts:
+      - exists:
+          path: metadata.labels
+      - isSubset:
+          path: metadata.labels
+          content:
+            LABEL: LABEL_V
+      - exists:
+          path: metadata.annotations
+      - isSubset:
+          path: metadata.annotations
+          content:
+            ANNOTATION: ANNOTATION_V
+  - it: Amoro Ingress should contains ingress annotations if set
+    set:
+      ingress:
+        enabled: true
+        annotations:
+          ANNOTATION: ANNOTATION_V2
+    asserts:
+      - exists:
+          path: metadata.annotations
+      - isSubset:
+          path: metadata.annotations
+          content:
+            ANNOTATION: ANNOTATION_V2
+  - it: Amoro Ingress should not contains annotations when annotations not set
+    set:
+      ingress:
+        enabled: true
+    asserts:
+      - notExists:
+          path: metadata.annotations
+  - it: Amoro Ingress should not contains ingress className if not set

Review Comment:
   fixed.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

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


Re: [PR] [AMORO-2778][Helm Chart] Add Tests for Helm Chart [amoro]

Posted by "lianneli (via GitHub)" <gi...@apache.org>.
lianneli commented on PR #2794:
URL: https://github.com/apache/amoro/pull/2794#issuecomment-2084703064

   > This PR is valuable, and now that we already have unittest, why not add it to the workflow? Would you be interested in completing this part in this PR?
   > 
   > @lianneli
   
   Thanks for your advice. I have add a workflow to run unit tests when change helm chart files. Could you help to check if it is proper?
   
   @baiyangtx 


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

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


Re: [PR] [AMORO-2778][Helm Chart] Add Tests for Helm Chart [amoro]

Posted by "lianneli (via GitHub)" <gi...@apache.org>.
lianneli commented on code in PR #2794:
URL: https://github.com/apache/amoro/pull/2794#discussion_r1589337193


##########
.github/workflows/pull-request-on-helmchat.yml:
##########
@@ -0,0 +1,56 @@
+#
+# Licensed to the 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.
+# The 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 workflow will execute Tests when submit a PR.
+
+name: "Pull Request Test For Helm Charts"
+

Review Comment:
   fixed.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@amoro.apache.org

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