You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@celeborn.apache.org by "ChenYi015 (via GitHub)" <gi...@apache.org> on 2024/05/15 02:58:41 UTC
[PR] [CELEBORN-1425][HELM] Add helm chart unit tests to ensure manifests are rendered as expected [celeborn]
ChenYi015 opened a new pull request, #2511:
URL: https://github.com/apache/celeborn/pull/2511
<!--
Thanks for sending a pull request! Here are some tips for you:
- Make sure the PR title start w/ a JIRA ticket, e.g. '[CELEBORN-XXXX] Your PR title ...'.
- Be sure to keep the PR description updated to reflect all changes.
- Please write your PR title to summarize what this PR proposes.
- If possible, provide a concise example to reproduce the issue for a faster review.
-->
### What changes were proposed in this pull request?
Add helm chart unit tests.
### Why are the changes needed?
Unit tests can make resource manifests are rendered as expected with various configurations.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Detailed information about how to run helm chart unit tests can be found here [helm-unittest/helm-unittest](https://github.com/helm-unittest/helm-unittest). First, you need to install helm unit test plugin:
```shell
helm plugin install https://github.com/helm-unittest/helm-unittest.git
```
Then, run helm chart unitt tests as follows:
```shell
$ helm unittest charts/celeborn --file "tests/**/*_test.yaml" --strict --debug
load_plugins.go:110: [info] file (/Users/chenyi/Library/helm/plugins/helm-acr/completion.yaml) not provided by plugin. No plugin auto-completion possible
### Chart [ celeborn ] charts/celeborn
PASS Test Celeborn configmap charts/celeborn/tests/configmap_test.yaml
PASS Test Celeborn master pod monitor charts/celeborn/tests/master/podmonitor_test.yaml
PASS Test Celeborn master priority class charts/celeborn/tests/master/priorityclass_test.yaml
PASS Test Celeborn master service charts/celeborn/tests/master/service_test.yaml
PASS Test Celeborn master statefulset charts/celeborn/tests/master/statefulset_test.yaml
PASS Test Celeborn worker pod monitor charts/celeborn/tests/worker/podmonitor_test.yaml
PASS Test Celeborn worker priority class charts/celeborn/tests/worker/priorityclass_test.yaml
PASS Test Celeborn worker service charts/celeborn/tests/worker/service_test.yaml
PASS Test Celeborn worker statefulset charts/celeborn/tests/worker/statefulset_test.yaml
Charts: 1 passed, 1 total
Test Suites: 9 passed, 9 total
Tests: 48 passed, 48 total
Snapshot: 0 passed, 0 total
Time: 183.011375ms
```
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1425][HELM] Add helm chart unit tests to ensure manifests are rendered as expected [celeborn]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #2511:
URL: https://github.com/apache/celeborn/pull/2511#issuecomment-2112250328
Thanks, merged to main for 0.5
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1425][HELM] Add helm chart unit tests to ensure manifests are rendered as expected [celeborn]
Posted by "ChenYi015 (via GitHub)" <gi...@apache.org>.
ChenYi015 commented on PR #2511:
URL: https://github.com/apache/celeborn/pull/2511#issuecomment-2111522566
@pan3793 I had added deverloper docs for running helm chart unit tests.
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1425][HELM] Add helm chart unit tests to ensure manifests are rendered as expected [celeborn]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #2511:
URL: https://github.com/apache/celeborn/pull/2511#issuecomment-2111762756
Also cc @RexXiong
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1425][HELM] Add helm chart unit tests to ensure manifests are rendered as expected [celeborn]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #2511:
URL: https://github.com/apache/celeborn/pull/2511#issuecomment-2111502286
could you please also write dev docs to guide contributors on how to run unit test? and set up CI for that? You can do it in dedicated PR or do it in place
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1425][HELM] Add helm chart unit tests to ensure manifests are rendered as expected [celeborn]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #2511:
URL: https://github.com/apache/celeborn/pull/2511#discussion_r1600973553
##########
docs/developers/helm-charts.md:
##########
@@ -0,0 +1,53 @@
+---
+license: |
+ 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
+
+ https://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.
+---
+
+# Helm Charts
+
+## Run Helm Unit Tests
+
+To run unit tests against Helm charts, first you need to install the helm unittest plugin as follows:
+
+```shell
+helm plugin install https://github.com/helm-unittest/helm-unittest.git
Review Comment:
Does that mean always using the latest version? it's fragile
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1425][HELM] Add helm chart unit tests to ensure manifests are rendered as expected [celeborn]
Posted by "ChenYi015 (via GitHub)" <gi...@apache.org>.
ChenYi015 commented on PR #2511:
URL: https://github.com/apache/celeborn/pull/2511#issuecomment-2111504305
> could you please also write dev docs to guide contributors on how to run unit test? and set up CI for that? You can do it in dedicated PR or do it in place
Ok, I will update docs in place and setup CI in a dedicated 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.
To unsubscribe, e-mail: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1425][HELM] Add helm chart unit tests to ensure manifests are rendered as expected [celeborn]
Posted by "ChenYi015 (via GitHub)" <gi...@apache.org>.
ChenYi015 commented on code in PR #2511:
URL: https://github.com/apache/celeborn/pull/2511#discussion_r1601051175
##########
docs/developers/helm-charts.md:
##########
@@ -0,0 +1,53 @@
+---
Review Comment:
@pan3793 Updated.
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1425][HELM] Add helm chart unit tests to ensure manifests are rendered as expected [celeborn]
Posted by "ChenYi015 (via GitHub)" <gi...@apache.org>.
ChenYi015 commented on PR #2511:
URL: https://github.com/apache/celeborn/pull/2511#issuecomment-2111499672
@pan3793 PTAL, thanks!
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1425][HELM] Add helm chart unit tests to ensure manifests are rendered as expected [celeborn]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 closed pull request #2511: [CELEBORN-1425][HELM] Add helm chart unit tests to ensure manifests are rendered as expected
URL: https://github.com/apache/celeborn/pull/2511
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1425][HELM] Add helm chart unit tests to ensure manifests are rendered as expected [celeborn]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #2511:
URL: https://github.com/apache/celeborn/pull/2511#discussion_r1600975210
##########
docs/developers/helm-charts.md:
##########
@@ -0,0 +1,53 @@
+---
Review Comment:
when adding new pages, `mkdocs.yml` should be updated too.
https://celeborn.apache.org/community/contributor_guide/docs_and_website/
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1425][HELM] Add helm chart unit tests to ensure manifests are rendered as expected [celeborn]
Posted by "ChenYi015 (via GitHub)" <gi...@apache.org>.
ChenYi015 commented on code in PR #2511:
URL: https://github.com/apache/celeborn/pull/2511#discussion_r1601050581
##########
docs/developers/helm-charts.md:
##########
@@ -0,0 +1,53 @@
+---
+license: |
+ 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
+
+ https://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.
+---
+
+# Helm Charts
+
+## Run Helm Unit Tests
+
+To run unit tests against Helm charts, first you need to install the helm unittest plugin as follows:
+
+```shell
+helm plugin install https://github.com/helm-unittest/helm-unittest.git
Review Comment:
> Does that mean always using the latest version? it's fragile
@pan3793 Yes, have updated to use fixed version `0.5.1`.
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CELEBORN-1425][HELM] Add helm chart unit tests to ensure manifests are rendered as expected [celeborn]
Posted by "ChenYi015 (via GitHub)" <gi...@apache.org>.
ChenYi015 commented on PR #2511:
URL: https://github.com/apache/celeborn/pull/2511#issuecomment-2111753418
@pan3793 I have updated the docs, PTAL, thanks.
--
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: issues-unsubscribe@celeborn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org