Skip to content

ACS: remove the old smart create templates and a few more improvements#2801

Merged
tjprescott merged 7 commits intoAzure:masterfrom
yugangw-msft:nonfat
Apr 11, 2017
Merged

ACS: remove the old smart create templates and a few more improvements#2801
tjprescott merged 7 commits intoAzure:masterfrom
yugangw-msft:nonfat

Conversation

@yugangw-msft
Copy link
Copy Markdown
Contributor

//cc: @brendandburns
Some addition change

  1. Move the e2e test from vm module to acs. We should have done that a while ago
  2. For consistency: support async create by exposing --no-wait argument and az acs wait command
  3. For consistency: expose --validate for dry-run test and you can use '--verbose' to see the full templage

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

General Guidelines

  • The PR has modified HISTORY.rst with an appropriate description of the change (see Modifying change log).

Command Guidelines

  • [] Each command and parameter has a meaningful description.
  • [] Each new command has a test.

(see Authoring Command Modules)

@codecov-io
Copy link
Copy Markdown

codecov-io commented Apr 7, 2017

Codecov Report

Merging #2801 into master will decrease coverage by 0.05%.
The diff coverage is 72%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2801      +/-   ##
==========================================
- Coverage   62.89%   62.84%   -0.06%     
==========================================
  Files         480      463      -17     
  Lines       26012    25822     -190     
  Branches     3946     3932      -14     
==========================================
- Hits        16361    16227     -134     
+ Misses       8627     8581      -46     
+ Partials     1024     1014      -10
Impacted Files Coverage Δ
...-cli-acs/azure/cli/command_modules/acs/commands.py 100% <100%> (ø) ⬆️
...e-cli-acs/azure/cli/command_modules/acs/_params.py 68.51% <100%> (+0.59%) ⬆️
...li-core/azure/cli/core/test_utils/vcr_test_base.py 70.58% <100%> (ø) ⬆️
...re-cli-acs/azure/cli/command_modules/acs/custom.py 38.61% <65%> (+3.59%) ⬆️
...s/azure/cli/command_modules/acs/_client_factory.py 60.86% <0%> (+34.78%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update de6fe79...eefd7a7. Read the comment docs.

Copy link
Copy Markdown
Member

@tjprescott tjprescott left a comment

Choose a reason for hiding this comment

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

LGTM. These FC -> ARM refactorings rely heavily on the fact that the test scenarios have to be re-recorded. If @brendandburns is satisfied that the tests cover the relevant scenarios, then I'm good. If not, we probably need to beef up the scenario test coverage to ensure this doesn't introduce regressions.

@yugangw-msft
Copy link
Copy Markdown
Contributor Author

@brendandburns, the test_acs_create_update was re-recorded to ensure non kubernetes acs was created correctly. I also manually ran the command to create a kubernetes acs. I can capture that scenario to a new test if you can tell me the property list to verify.

@brendandburns
Copy link
Copy Markdown
Member

brendandburns commented Apr 7, 2017 via email

Copy link
Copy Markdown
Member

@tjprescott tjprescott left a comment

Choose a reason for hiding this comment

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

Thanks for adding the second scenario test. LGTM. Bye bye "smart create"!

@tjprescott tjprescott merged commit b3b1b4e into Azure:master Apr 11, 2017
@yugangw-msft yugangw-msft deleted the nonfat branch April 24, 2017 16:51
00Kai0 pushed a commit to 00Kai0/azure-cli that referenced this pull request Apr 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants