azure-cli-extensions icon indicating copy to clipboard operation
azure-cli-extensions copied to clipboard

K8s extension/release 1.3.6

Open deeksha345 opened this issue 1 year ago • 13 comments


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

Related command

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.
The precondition is to put your code inside this repository and upgrade the version in the pull request but do not modify src/index.json.

deeksha345 avatar Oct 06 '22 20:10 deeksha345

K8s extension

yonzhan avatar Oct 06 '22 21:10 yonzhan

/azp run

wangzelin007 avatar Oct 07 '22 06:10 wangzelin007

Azure Pipelines successfully started running 1 pipeline(s).

azure-pipelines[bot] avatar Oct 07 '22 06:10 azure-pipelines[bot]

All checks are passing can I please get this PR approved and merged in, thanks!

deeksha345 avatar Oct 07 '22 17:10 deeksha345

Could you add some test cases for those new features?

zhoxing-ms avatar Oct 09 '22 03:10 zhoxing-ms

Could you add some test cases for those new features?

Which features are you talking about? I already have test cases for the extension type calls.

deeksha345 avatar Oct 10 '22 19:10 deeksha345

Which features are you talking about? I already have test cases for the extension type calls

@deeksha345 This PR does not seem to contain tests. Could you show me the code link of tests?

Fix the TypeError: cf_k8s_extension() takes 1 positional argument but 2 were given while running all az k8s-extension extension-types commands

I believe that such problems should be covered by tests

zhoxing-ms avatar Oct 11 '22 02:10 zhoxing-ms

I have added tests as part of our custom pipeline in our forked repo and any changes we push to upstream repo are only after all those checks pass. Those files are deleted and not included in our PRs to the upstream repo though. Here is the link to the tests in our repo: https://github.com/AzureArcForKubernetes/azure-cli-extensions/blob/k8s-extension/public/testing/test/extensions/public/ExtensionTypes.Tests.ps1

deeksha345 avatar Oct 11 '22 22:10 deeksha345

@deeksha345 Could you add scenario tests in azure-cli-extensions repo? In this way, the CI pipeline of CLI extension repo can also help you perform regression testing when you are submitting PRs

zhoxing-ms avatar Oct 12 '22 02:10 zhoxing-ms

We have tests in our forked repo and Bavneet said that is satisfactory for providing test coverage overall. We do not update the tests in the azure cli repo. If I added tests also in azure-cli-extensions also I would just be adding the same tests twice and I do not see the point in that as our custom pipeline already provides the test coverage needed.

deeksha345 avatar Oct 12 '22 17:10 deeksha345

Ok this is what Narayan said - These tests will be used during Official CLI releases and in their Nightly tests, which do not use our pipeline. So, yes, you will have to create tests. For our scenarios, since we require a K8s cluster to be created, and since it is not possible to create that in their runs we record these tests once and mark them such that they are just replayed and not run. All our existing tests are marked this way. Refer to the Az CLI Authoring documentation on how to mark tests for replay only.

@zhoxing-ms Can you point me to the documentation that talks about how to mark tests as replay only as I cannot find any information about it.

deeksha345 avatar Oct 13 '22 20:10 deeksha345

We have tests in our forked repo and Bavneet said that is satisfactory for providing test coverage overall. We do not update the tests in the azure cli repo. If I added tests also in azure-cli-extensions also I would just be adding the same tests twice and I do not see the point in that as our custom pipeline already provides the test coverage needed.

@deeksha345 This is convenient for us to maintain test cases with the source code, and we can find problems as soon as possible when submitting PR. If problems are found in the tests, the PR merge can be blocked in time

zhoxing-ms avatar Oct 14 '22 02:10 zhoxing-ms

@deeksha345 Please refer to this doc authoring_tests.md to write tests

zhoxing-ms avatar Oct 14 '22 03:10 zhoxing-ms

We have tests in our forked repo and Bavneet said that is satisfactory for providing test coverage overall. We do not update the tests in the azure cli repo. If I added tests also in azure-cli-extensions also I would just be adding the same tests twice and I do not see the point in that as our custom pipeline already provides the test coverage needed.

@deeksha345 This is convenient for us to maintain test cases with the source code, and we can find problems as soon as possible when submitting PR. If problems are found in the tests, the PR merge can be blocked in time

@zhoxing-ms I have added the tests please take a look and merge this PR if everything looks good!

deeksha345 avatar Oct 18 '22 17:10 deeksha345

[Release] Update index.json for extension [ k8s-extension ] : https://dev.azure.com/azclitools/internal/_build/results?buildId=10042&view=results

azclibot avatar Oct 20 '22 03:10 azclibot

For 2 of these test scenarios - the show and the list with cluster - we require a K8s cluster to be created, and since it is not possible to create that in your runs I marked these tests as record only.

deeksha345 avatar Oct 20 '22 17:10 deeksha345