Skip to content

[containerapp]az containerapp compose create: Fix environment resource group parsing issue and improve test cases#6807

Merged
wangzelin007 merged 12 commits intoAzure:mainfrom
Greedygre:xinyu/containerapp_refine_test_env
Oct 8, 2023
Merged

[containerapp]az containerapp compose create: Fix environment resource group parsing issue and improve test cases#6807
wangzelin007 merged 12 commits intoAzure:mainfrom
Greedygre:xinyu/containerapp_refine_test_env

Conversation

@Greedygre
Copy link
Contributor

@Greedygre Greedygre commented Sep 26, 2023


This checklist is used to make sure that common guidelines for a pull request are followed.

Related command

az containerapp compose create

General Guidelines

  • Have you run azdev style <YOUR_EXT> locally? (pip install azdev required)
  • Have you run python scripts/ci/test_index.py -q locally?

For new extensions:

About Extension Publish

There is a pipeline to automatically build, upload and publish extension wheels.
Once your pull request is merged into main branch, a new pull request will be created to update src/index.json automatically.
You only need to update the version information in file setup.py and historical information in file HISTORY.rst in your PR but do not modify src/index.json.

@azure-client-tools-bot-prd
Copy link

azure-client-tools-bot-prd bot commented Sep 26, 2023

️✔️Azure CLI Extensions Breaking Change Test
️✔️Non Breaking Changes

@azure-client-tools-bot-prd
Copy link

Hi @Greedygre,
Please write the description of changes which can be perceived by customers into HISTORY.rst.
If you want to release a new extension version, please update the version in setup.py as well.

@yonzhan
Copy link
Collaborator

yonzhan commented Sep 26, 2023

containerapp

@Greedygre Greedygre changed the title {containerapp} Improve test cases [containerapp]az containerapp compose create: Fix environment resource group parsing issue and improve test cases Sep 26, 2023
@Greedygre Greedygre marked this pull request as ready for review October 7, 2023 04:37
Comment on lines +18 to +19
rg_name = f'client.env_rg_{TEST_LOCATION}'.lower().replace(" ", "").replace("(", "").replace(")", "")
env_name = f'env-{TEST_LOCATION}'.lower().replace(" ", "").replace("(", "").replace(")", "")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we run tests in parallel (recording tests and live tests) and different test cases use the same rg_name and env_name, will there be test failures?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For testing app scenarios, let them use the same env in the same region to test without having to create a new environment every time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Different test app has different app name, so they can pass without conflict.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will result in a remaining resource group that stores env.
Let's merge this PR first, I will consider deleting the resource group in our custom cleanup script.

@wangzelin007 wangzelin007 merged commit 082e4b9 into Azure:main Oct 8, 2023
scrappywyrm pushed a commit to scrappywyrm/azure-cli-extensions that referenced this pull request Oct 16, 2023
…rce group parsing issue and improve test cases (Azure#6807)

* add prepare

* fix compose create

* containerapp tests use same environment

* add prepare

* fix compose create

* add history for fix az containerapp compose creste

* tests for containerapp use same env and rerun tests

* change to env rg

* remove unused import

* comment addressed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Auto-Assign Auto assign by bot ContainerApp

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants