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

Refactor string normalization function and test it. #35

Closed
HarikrishnanBalagopal opened this issue Oct 3, 2020 · 3 comments · Fixed by #42
Closed

Refactor string normalization function and test it. #35

HarikrishnanBalagopal opened this issue Oct 3, 2020 · 3 comments · Fixed by #42
Assignees
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. hacktoberfest

Comments

@HarikrishnanBalagopal
Copy link
Contributor

Description

This package normalizes certain strings in the intermediate representation.
It strips any whitespace and quotations/commas from each container's environment variables.

Two things need to be done:

  1. The package is not written in the most idiomatic way.
  2. The package needs unit tests.

Some examples of code that needs to be refactored:

How to get started

For this issue you should start with some simple tests that creates simple IR objects and calls the function on them.
Next step would be to make slight changes to the IR objects and create a separate subtest for each scenario.
Some scenarios that are good to test:

  • Function called on an IR with no services.
  • An IR containing services that have no containers.
  • An IR containing services and containers but the containers have no environment variables.
  • An IR containing services and containers and all the environment variables are valid.
  • An IR containing services and containers and some of the environment variables are invalid (containing spaces and quotes).
  • An IR containing services and containers and some of the environment variables are invalid but their names contain the string affinity.

How to add unit tests

Some guidelines:

  • Look at the other tests in the package/project and write similar tests.
  • Use subtests to test different paths through the function.
  • Don't worry about testing every single path through the function. Focus on common use cases and failure modes.

Some helpful resources on how to write unit tests in Go:

Code to be tested

func (po normalizeCharacterOptimizer) optimize(ir irtypes.IR) (irtypes.IR, error) {
//TODO: Make this generic to ensure all fields have valid names
for k := range ir.Services {
scObj := ir.Services[k]
for _, serviceContainer := range scObj.Containers {
var tmpEnvArray []corev1.EnvVar
for _, env := range serviceContainer.Env {
if !strings.Contains(env.Name, "affinity") {
env.Name = strings.Trim(env.Name, "\t \n")
env.Value = strings.Trim(env.Value, "\t \n")
tmpString, err := stripQuotation(env.Name)
if err == nil {
env.Name = tmpString
}
tmpString, err = stripQuotation(env.Value)
if err == nil {
env.Value = tmpString
}
tmpEnvArray = append(tmpEnvArray, env)
}
}
serviceContainer.Env = tmpEnvArray
}
ir.Services[k] = scObj
}
return ir, nil
}
func stripQuotation(inputString string) (string, error) {
regex := regexp.MustCompile(`^[',"](.*)[',"]$`)
return regex.ReplaceAllString(inputString, `$1`), nil
}

@HarikrishnanBalagopal HarikrishnanBalagopal added good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. hacktoberfest labels Oct 3, 2020
@suramrit
Copy link

suramrit commented Oct 9, 2020

@HarikrishnanBalagopal Would like to get working on this!
Thanks for the great head start and background on the issue! 🚀

@ShajithaMohammed
Copy link

@HarikrishnanBalagopal @ashokponkumar @suramrit Actually I have addressed this issue and ready to raise a PR,Forgot to sign-up. Apologies.
Let me know if I can go ahead and raise a PR.

@ashokponkumar
Copy link
Member

@ShajithaMohammed You can raise a PR and together with @suramrit based on his changes, we can together refine it and merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. hacktoberfest
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants