-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added L0 test cases for kubernetes #6494
Conversation
Tasks/Kubernetes/Tests/TestSetup.ts
Outdated
fsClone.existsSync = function(filePath) { | ||
switch (filePath) { | ||
case "kubectl": | ||
if(JSON.parse(process.env[shared.isKubectlPresentOnMachine])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why JSON.parse
Tasks/Kubernetes/Tests/L0.ts
Outdated
}); | ||
|
||
it('Run sucessfully when the user provides the version for kubectl with checkLatest as false', (done:MochaDone) => { | ||
let tp = path.join(__dirname, 'TestSetup.js'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no check, in the test that kubectl is downloaded and version 1.7 is downloaded.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
applied check
Tasks/Kubernetes/Tests/L0.ts
Outdated
assert(tr.invokedToolCount == 1, 'should have invoked tool one times. actual: ' + tr.invokedToolCount); | ||
assert(tr.stderr.length == 0 || tr.errorIssues.length, 'should not have written to stderr'); | ||
assert(tr.succeeded, 'task should have succeeded'); | ||
assert(tr.stdout.indexOf(`[command]${shared.formatPath("newUserDir/kubectl.exe")} --kubeconfig ${shared.formatPath("newUserDir/config")} get pods`) != -1, "kubectl apply should run"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
command you are trying to run is get pods not apply. Please fix the message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed message
Tasks/Kubernetes/Tests/L0.ts
Outdated
}); | ||
|
||
it('Run sucessfully when the user provides the version for kubectl with checkLatest as true', (done:MochaDone) => { | ||
let tp = path.join(__dirname, 'TestSetup.js'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can not see any check that latest version is looked for and downloded.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added check
Tasks/Kubernetes/Tests/L0.ts
Outdated
assert(tr.invokedToolCount == 1, 'should have invoked tool one times. actual: ' + tr.invokedToolCount); | ||
assert(tr.stderr.length == 0 || tr.errorIssues.length, 'should not have written to stderr'); | ||
assert(tr.succeeded, 'task should have succeeded'); | ||
assert(tr.stdout.indexOf(`[command]${shared.formatPath("newUserDir/kubectl.exe")} --kubeconfig ${shared.formatPath("newUserDir/config")} get pods`) != -1, "kubectl apply should run"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
command is get pods and message is apply. This error is in all test please fix it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Tasks/Kubernetes/Tests/L0.ts
Outdated
after(function () { | ||
}); | ||
|
||
it('Run sucessfully when the user provides a specific location for kubectl', (done:MochaDone) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
type in sucessfully => successfully in every test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Tasks/Kubernetes/Tests/L0.ts
Outdated
assert(tr.invokedToolCount == 1, 'should have invoked tool one times. actual: ' + tr.invokedToolCount); | ||
assert(tr.stderr.length == 0 || tr.errorIssues.length, 'should not have written to stderr'); | ||
assert(tr.succeeded, 'task should have succeeded'); | ||
assert(tr.stdout.indexOf(`[command]${shared.formatPath("newUserDir/kubectl.exe")} --kubeconfig ${shared.formatPath("newUserDir/config")} get pods`) != -1, "kubectl apply should run"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you copied the checks in output section as is in all test cases. There is no check to validate that, tests are running the way they are expected. Tests might succeed but does not execute the way they are supposed to.
Tasks/Kubernetes/Tests/L0.ts
Outdated
done(); | ||
}); | ||
|
||
it('Runs successfully for kubectl exec', (done:MochaDone) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We dont have to add test cases for each command.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed duplicate test cases
Tasks/Kubernetes/Tests/L0.ts
Outdated
done(); | ||
}); | ||
|
||
it('Runs successfully for kubectl top', (done:MochaDone) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same comment, tests for each command is duplicate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed duplicate test cases
Tasks/Kubernetes/Tests/TestSetup.ts
Outdated
return "v1.6.6"; | ||
}, | ||
getKubectlVersion: async function(versionSpec, checkLatest) : Promise<string> { | ||
let version: string = "v1.6.6"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should avoid having logic in mocq.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to add the logic as the mock getStableKubectlVersion() will not be called if we use the original getKubectlVersion().
Tasks/Kubernetes/Tests/L0.ts
Outdated
assert(tr.succeeded, 'task should have succeeded'); | ||
assert(tr.stdout.indexOf(`Get stable kubectl version`) != -1, "Stable version of kubectl is downloaded"); | ||
assert(tr.stdout.indexOf(`Got kubectl version v1.6.6`) != -1, "Got the latest version of kubectl"); | ||
assert(tr.stdout.indexOf(`Downloaded kubectl`) != -1, "Downloaded correct version of kubectl"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should check that the version is 1.7.0 and not anything else. I am pushing hard to have our test correct because it will help us in future to push update fast and not have to test on windows and linux separately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
assert(tr.stderr.length == 0 || tr.errorIssues.length, 'should not have written to stderr'); | ||
assert(tr.succeeded, 'task should have succeeded'); | ||
assert(tr.stdout.indexOf(`Get stable kubectl version`) != -1, "Stable version of kubectl is downloaded"); | ||
assert(tr.stdout.indexOf(`Got kubectl version v1.6.6`) != -1, "Got the latest version of kubectl"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does output contains 'Got kubectl version v1.6.6', it should be 1.7.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When version is 1.7 we check for the stable latest version which is 1.6.6
Tasks/Kubernetes/Tests/L0.ts
Outdated
assert(tr.invokedToolCount == 2, 'should have invoked tool one times. actual: ' + tr.invokedToolCount); | ||
assert(tr.stderr.length == 0 || tr.errorIssues.length, 'should not have written to stderr'); | ||
assert(tr.succeeded, 'task should have succeeded'); | ||
assert(tr.stdout.indexOf(`[command]kubectl --kubeconfig ${shared.formatPath("newUserDir/config")} create secret docker-registry my-secret --docker-server=https://index.docker.io/v1/ --docker-username=test --docker-password=regpassword [email protected]`) != -1, "kubectl create should run"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is no check of forceUpdate here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added check for delete secret
Tasks/Kubernetes/Tests/L0.ts
Outdated
assert(tr.invokedToolCount == 2, 'should have invoked tool one times. actual: ' + tr.invokedToolCount); | ||
assert(tr.stderr.length == 0 || tr.errorIssues.length, 'should not have written to stderr'); | ||
assert(tr.succeeded, 'task should have succeeded'); | ||
assert(tr.stdout.indexOf(`[command]kubectl --kubeconfig ${shared.formatPath("newUserDir/config")} create secret docker-registry my-secret --docker-server=ajgtestacr1.azurecr.io --docker-username=spId --docker-password=spKey --docker-email=ServicePrincipal@AzureRM`) != -1, "kubectl create should run"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again no mention of force update in the command.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added
84e2301
to
5b1c612
Compare
No description provided.