Skip to content
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

Add multi-namespace checks to the deploy tests #65

Closed
LucasRoesler opened this issue Apr 8, 2021 · 2 comments · Fixed by #66
Closed

Add multi-namespace checks to the deploy tests #65

LucasRoesler opened this issue Apr 8, 2021 · 2 comments · Fixed by #66

Comments

@LucasRoesler
Copy link
Member

Continuing with #61, let's add tests that verify that deploying a function works when multiple namespaces are enabled. A couple things we would really like to have

  1. we still want to test when the namespace value is omitted, this is the implied "deploy to default namespace"
  2. we probably don't need to test all of the non-default namespaces, so we are probably ok with just testing in the first value of config.Namespaces
  3. ideally, these additional tests should be controlled by the config.Namespaces so that simply providing the CERTIFIER_NAMESPACES is enough to enable the additional tests. Similarly, if CERTIFIER_NAMESPACES is empty or does not exist, then it only tests the implied default namespace, see (1) above. Ideally, this means we do not need any additional configuration to control this.
  4. the test Test_Deploy_PassingCustomEnvVars_AndQueryString actually looks like it should be in the invoke_test.go let's move and rename that appropriately. It isn't actually testing the deployment or configuration of a Function, rather it is testing the support for GET parameters during invoke.
  5. Similarly, this since we have a separate file/flow for testing invoking, we don't need to have any function invocation in the deploy tests. For example, i think we can omit this line
    _ = invoke(t, functionRequest.FunctionName, emptyQueryString, http.StatusOK)
  6. Finally, one last improvement we could make to the tests is to consolidate the Test_Deploy_Stronghash, Test_Deploy_WithLabels, and Test_Deploy_WithLabels into a single table driven test. We can improve the tests in general by just always checking the name, labels, annotations, etc. For example, it is valid to verify that if the test case has an empty label set, then the deploy response should still be empty.

Putting all of that together, i think we end up with a single table-driven test TestDeploy where we have a sub-test for (at the minimum)

  • the minimal deployment, default and non-default namespace
  • deployment with non-empty labels, default and non-default namespace
  • deployment with non-empty annotations, default and non-default namespace

We can probably create this by creating the initial list of test cases

cases := []struct{
   name string
   req sdk.DeployFunctionSpec
}{
   // cases here 
}

and then we can just loop over it to add a copy of each test with the non-default namespace

defaultCases := len(cases)
for i := 0; i < defaultCases; i++ {
    newCase := cases[i]
    // rename the case
    // set the req Namespace
    cases = append(cases, newCase)
}
@LucasRoesler
Copy link
Member Author

@nitishkumar71 what do you think?

@nitishkumar71
Copy link
Member

Thanks for briefing it, looks correct to me. I Will start working on it in some time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants