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

Added Unit test coverage for Kustomize V3 Iac-provider #399

Merged
merged 1 commit into from
Dec 1, 2020

Conversation

devang-gaur
Copy link
Contributor

Fix #379

@codecov
Copy link

codecov bot commented Nov 21, 2020

Codecov Report

Merging #399 (1f296e6) into master (d37fb58) will increase coverage by 2.66%.
The diff coverage is 90.90%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #399      +/-   ##
==========================================
+ Coverage   63.55%   66.21%   +2.66%     
==========================================
  Files          85       85              
  Lines        1904     1915      +11     
==========================================
+ Hits         1210     1268      +58     
+ Misses        591      535      -56     
- Partials      103      112       +9     
Impacted Files Coverage Δ
pkg/iac-providers/kustomize/v3/load-dir.go 73.07% <83.33%> (+73.07%) ⬆️
pkg/utils/resource.go 17.64% <100.00%> (+17.64%) ⬆️
pkg/utils/yaml.go 71.92% <100.00%> (+10.11%) ⬆️
...iac-providers/terraform/v12/variable-references.go 58.33% <0.00%> (-6.49%) ⬇️
pkg/utils/path.go 90.47% <0.00%> (+11.90%) ⬆️
pkg/iac-providers/kustomize/v3/types.go 100.00% <0.00%> (+100.00%) ⬆️
pkg/iac-providers/kustomize/v3/load-file.go 100.00% <0.00%> (+100.00%) ⬆️

Copy link
Contributor

@kanchwala-yusuf kanchwala-yusuf left a comment

Choose a reason for hiding this comment

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

I believe, there is scope for adding test cases for testing various error conditions.
For eg:

  • In LoadIacDir, a test case can be added to check the error when kustomization.yaml is not present in the user provided directory
  • Similarly, a test case for checking the error conditions when more than 1 kustomization.yamls are present.
  • Another, test case is possible for an empty kustomization.yaml

In this way some test cases can be added for LoadKustomize method as well

@devang-gaur
Copy link
Contributor Author

devang-gaur commented Nov 22, 2020

I believe, there is scope for adding test cases for testing various error conditions.
For eg:

  • In LoadIacDir, a test case can be added to check the error when kustomization.yaml is not present in the user provided directory

Thanks for reminding. Missed that case.

Similarly, a test case for checking the error conditions when more than 1 kustomization.yamls are present.

you can't have more than one file with the same name in a directory (as per OS constraints). So that part is taken care of
already.

Realised later : There can be kustomization.yaml and kustomization.yml file in the same directory!

Another, test case is possible for an empty kustomization.yaml

Yep

wantErr error
}{
{
name: "load iac file is not supported for helm",
Copy link
Contributor

Choose a reason for hiding this comment

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

"for kustomize"

zap.S().Error("error while searching for iac files", zap.String("root dir", absRootDir), zap.Error(err))
return allResourcesConfig, err
}

if len(files) == 0 {
err = errors.New("could not find a kustomization.yaml/yml file in the directory")
err = errorKustomizeNotFound(errors.New("could not find a kustomization.yaml/yml file in the directory"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Recommend to move the error definitions into a variable that can be referenced easily by unit tests

zap.S().Error("error while searching for iac files", zap.String("root dir", absRootDir), zap.Error(err))
return allResourcesConfig, err
}

if len(files) > 1 {
err = errors.New("a directory cannot have more than 1 kustomization.yaml/yml file")
err = errorMultipleKustomizeFile(errors.New("a directory cannot have more than 1 kustomization.yaml/yml file"))
Copy link
Contributor

Choose a reason for hiding this comment

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

same--Recommend to move the error definitions into a variable that can be referenced easily by unit tests

err = fmt.Errorf("unable to read any kustomization file in the directory : %v", err)
zap.S().Error("error while searching for iac files", zap.String("root dir", absRootDir), zap.Error(err))
if err != nil {
err = fmt.Errorf("unable to read the kustomization file in the directory : %v", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Recommend to move the error definitions into a variable that can be referenced easily by unit tests

@sonarcloud
Copy link

sonarcloud bot commented Dec 1, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
3.5% 3.5% Duplication

@williepaul williepaul merged commit ab97a48 into tenable:master Dec 1, 2020
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 this pull request may close these issues.

Test coverage missing for kustomize iac-provider
3 participants