Re: [PR] KNOX-3228: New create-list-aliases integration test [knox]
hanicz closed pull request #1123: KNOX-3228: New create-list-aliases integration test URL: https://github.com/apache/knox/pull/1123 -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] KNOX-3228: New create-list-aliases integration test [knox]
moresandeep commented on PR #1123: URL: https://github.com/apache/knox/pull/1123#issuecomment-3631693206 Looks like the PR was opened before i merged my changes that replace [host name from knox to localhost](https://github.com/apache/knox/pull/1125/files#diff-895483e96b37ea49f8d4c82cf2e2b33500a5b34e99245ce5d8a0a6595f3e06b1R26) ``` tests-1 | host = 'knox', port = 8443, family = tests-1 | type = , proto = 0, flags = 0 ``` Notice the host name is knox it was fixed to localhost but the way this workflow runs the changes have to be in master. You can add `skip-tests` label to skip the test and commit. This should not be a problem going forward. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] KNOX-3228: New create-list-aliases integration test [knox]
hanicz commented on code in PR #1123: URL: https://github.com/apache/knox/pull/1123#discussion_r2601273520 ## .github/workflows/compose/docker-compose.yml: ## @@ -50,6 +50,7 @@ services: working_dir: /tests volumes: - ../tests:/tests + - /var/run/docker.sock:/var/run/docker.sock Review Comment: Yes -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] KNOX-3228: New create-list-aliases integration test [knox]
hanicz commented on code in PR #1123: URL: https://github.com/apache/knox/pull/1123#discussion_r2601273085 ## .github/workflows/tests/test_knox_cli.py: ## @@ -0,0 +1,44 @@ +# 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. +import unittest +from util.knox import Knox + Review Comment: Added a description to the file however the individual tests have their own descriptions. ## .github/workflows/tests/requirements.txt: ## @@ -1,2 +1,3 @@ requests +docker Review Comment: Done -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] KNOX-3228: New create-list-aliases integration test [knox]
Raghav-Mah3shwari commented on code in PR #1123: URL: https://github.com/apache/knox/pull/1123#discussion_r2599984902 ## .github/workflows/tests/requirements.txt: ## @@ -1,2 +1,3 @@ requests +docker Review Comment: can we also keep the dependencies sorted (ascending or descending) as well just to make this file a little more readable down the line when we might have a lot of dependencies. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] KNOX-3228: New create-list-aliases integration test [knox]
moresandeep commented on code in PR #1123: URL: https://github.com/apache/knox/pull/1123#discussion_r2599945196 ## .github/workflows/compose/docker-compose.yml: ## @@ -50,6 +50,7 @@ services: working_dir: /tests volumes: - ../tests:/tests + - /var/run/docker.sock:/var/run/docker.sock Review Comment: I see, so this is for workflow right? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] KNOX-3228: New create-list-aliases integration test [knox]
hanicz commented on code in PR #1123: URL: https://github.com/apache/knox/pull/1123#discussion_r2599817667 ## .github/workflows/compose/docker-compose.yml: ## @@ -50,6 +50,7 @@ services: working_dir: /tests volumes: - ../tests:/tests + - /var/run/docker.sock:/var/run/docker.sock Review Comment: We need this so for `self.knox_container = client.containers.get(container_name)` Without it the docker library can't find the container. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] KNOX-3228: New create-list-aliases integration test [knox]
moresandeep commented on code in PR #1123: URL: https://github.com/apache/knox/pull/1123#discussion_r2599802165 ## .github/workflows/tests/requirements.txt: ## @@ -1,2 +1,3 @@ requests +docker Review Comment: I have changes in this file, I am pinning version based on @Raghav-Mah3shwari 's input, that i will be committing. You will need to rebase, sorry :( ## .github/workflows/tests/test_knox_cli.py: ## @@ -0,0 +1,44 @@ +# 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. +import unittest +from util.knox import Knox + Review Comment: Test description would be nice! ## .github/workflows/compose/docker-compose.yml: ## @@ -50,6 +50,7 @@ services: working_dir: /tests volumes: - ../tests:/tests + - /var/run/docker.sock:/var/run/docker.sock Review Comment: Why do we need this? This looks like a change needed for local runs. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
[PR] KNOX-3228: New create-list-aliases integration test [knox]
hanicz opened a new pull request, #1123: URL: https://github.com/apache/knox/pull/1123 [KNOX-3228](https://issues.apache.org/jira/browse/KNOX-3228) - KnoxCLI integration test ## What changes were proposed in this pull request? - Adds a new integration test for create-list-aliases KnoxCLI command. - Adds the necessary steps and libraries to call KnoxCLI commands in integration tests. ## How was this patch tested? Ran the integration tests locally `docker compose -f ./.github/workflows/compose/docker-compose.yml up --exit-code-from tests 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
