Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

Unit tests #3344

Merged
merged 14 commits into from
Jul 5, 2018
Merged

Unit tests #3344

merged 14 commits into from
Jul 5, 2018

Conversation

CecileRobertMichon
Copy link
Contributor

@CecileRobertMichon CecileRobertMichon commented Jun 22, 2018

What this PR does / why we need it: improves unit test coverage

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): improves #2576

Special notes for your reviewer:

If applicable:

  • documentation
  • unit tests
  • tested backward compatibility (ie. deploy with previous version, upgrade with this branch)

Release note:

@codecov
Copy link

codecov bot commented Jun 23, 2018

Codecov Report

Merging #3344 into master will increase coverage by 0.53%.
The diff coverage is 93.97%.

@@            Coverage Diff            @@
##           master   #3344      +/-   ##
=========================================
+ Coverage   55.37%   55.9%   +0.53%     
=========================================
  Files         104     105       +1     
  Lines       15785   15855      +70     
=========================================
+ Hits         8741    8864     +123     
+ Misses       6325    6249      -76     
- Partials      719     742      +23

@@ -352,71 +351,3 @@ func TestAPIServerConfigDefaultAdmissionControls(t *testing.T) {
t.Fatalf("Admission control key '%s' not set in API server config for version %s", enableAdmissionPluginsKey, version)
}
}

func createContainerService(containerServiceName string, orchestratorVersion string, masterCount int, agentCount int) *api.ContainerService {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

refactored in common place, function was duplicated

@CecileRobertMichon CecileRobertMichon changed the title [WIP] unit tests Unit tests Jun 25, 2018
@@ -114,6 +114,9 @@ func (w *ArtifactWriter) WriteTLSArtifacts(containerService *api.ContainerServic
return e
}
for i := 0; i < properties.MasterProfile.Count; i++ {
if len(properties.CertificateProfile.EtcdPeerPrivateKeys) <= i || len(properties.CertificateProfile.EtcdPeerCertificates) <= i {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Guard against index out of bound

@@ -101,3 +103,86 @@ type KeyVaultRef struct {
}

type paramsMap map[string]interface{}

// CreateMockContainerService returns a mock container service for testing purposes
func CreateMockContainerService(containerServiceName, orchestratorVersion string, masterCount, agentCount int, certs bool) *api.ContainerService {
Copy link
Member

Choose a reason for hiding this comment

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

Let's move this func to another file, perhaps mocks.go?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -45,24 +45,7 @@ func NewRootCmd() *cobra.Command {
rootCmd.AddCommand(newUpgradeCmd())
rootCmd.AddCommand(newScaleCmd())
rootCmd.AddCommand(newDcosUpgradeCmd())

var completionCmd = &cobra.Command{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved this to a function, fyi @stuartleeks @jackfrancis

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool :-)

@jackfrancis
Copy link
Member

/lgtm

@acs-bot
Copy link

acs-bot commented Jul 5, 2018

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CecileRobertMichon, jackfrancis

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [CecileRobertMichon,jackfrancis]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jackfrancis jackfrancis merged commit a8cd0a4 into Azure:master Jul 5, 2018
@ghost ghost removed the in progress label Jul 5, 2018
PaulCharlton added a commit to ElementAnalytics/acs-engine that referenced this pull request Jul 7, 2018
* 'master' of https://github.com/Azure/acs-engine: (74 commits)
  Fix the network blip caused during network creation/deletion (Azure#3438)
  Adding e2e sample for kubenet+wincni and adding DNS back to e2e tests (Azure#3429)
  openshift: quote all values going into yaml files and shell scripts (Azure#3443)
  Update clusterKeyVaultName for kv and sa (Azure#3440)
  Kubernetes: Azure CNI v1.0.7 (Azure#3433)
  account for nil (Azure#3442)
  higher timeout, more retries for dpkg -i (Azure#3441)
  Reenable azurefile test (Azure#3402)
  triage non-working KMS test implementation (Azure#3437)
  Kubernetes: install blobfuse during CSE (Azure#3401)
  Fix KMS in multi api-server (Azure#3430)
  update azure-npm to v0.0.4 (Azure#3426)
  Kubernetes E2E: optimized HPA test (Azure#3428)
  Starting Windows troubleshooting steps (Azure#3431)
  pin device plugin ds to only labelled nodes. Update device plugin version for 1.11 (Azure#3422)
  E2E cleanup: "acse-test-infrastructure" is too long (Azure#3425)
  Remove DenyEscalatingExec admission controller from default list (Azure#3423)
  long-term DNS livenessProbe tests for soak cluster (Azure#3424)
  Unit tests (Azure#3344)
  remove version check from k8s e2e (Azure#3419)
  ...
PaulCharlton added a commit to ElementAnalytics/acs-engine that referenced this pull request Jul 7, 2018
* master: (566 commits)
  Fix the network blip caused during network creation/deletion (Azure#3438)
  Adding e2e sample for kubenet+wincni and adding DNS back to e2e tests (Azure#3429)
  openshift: quote all values going into yaml files and shell scripts (Azure#3443)
  Update clusterKeyVaultName for kv and sa (Azure#3440)
  Kubernetes: Azure CNI v1.0.7 (Azure#3433)
  account for nil (Azure#3442)
  higher timeout, more retries for dpkg -i (Azure#3441)
  Reenable azurefile test (Azure#3402)
  triage non-working KMS test implementation (Azure#3437)
  Kubernetes: install blobfuse during CSE (Azure#3401)
  Fix KMS in multi api-server (Azure#3430)
  update azure-npm to v0.0.4 (Azure#3426)
  Kubernetes E2E: optimized HPA test (Azure#3428)
  Starting Windows troubleshooting steps (Azure#3431)
  pin device plugin ds to only labelled nodes. Update device plugin version for 1.11 (Azure#3422)
  E2E cleanup: "acse-test-infrastructure" is too long (Azure#3425)
  Remove DenyEscalatingExec admission controller from default list (Azure#3423)
  long-term DNS livenessProbe tests for soak cluster (Azure#3424)
  Unit tests (Azure#3344)
  remove version check from k8s e2e (Azure#3419)
  ...
@CecileRobertMichon CecileRobertMichon deleted the cmd-unit-tests branch July 11, 2018 21:11
julienstroheker pushed a commit to julienstroheker/acs-engine that referenced this pull request Jul 16, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants