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

[feature]: Automatically add test reports folder to gitignore #1798

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

Akash-Singh04
Copy link
Contributor

Related Issue

  • Create a gitignore file in the keploy folder that adds the test report folder and prevents it from accidentally being commited.

Closes: #1775

Describe the changes you've made

  • Create a gitignore file in the keploy folder that adds the test report folder and prevents it from accidentally being commited.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, local variables)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Please let us know if any test cases are added

NIL
Please describe the tests(if any). Provide instructions how its affecting the coverage.
NIL

Describe if there is any unusual behaviour of your code(Write NA if there isn't)

A clear and concise description of it.
NIL

Checklist:

  • My code follows the style guidelines of this project.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.

Screenshots (if any)

image

@Akash-Singh04
Copy link
Contributor Author

@slayerjain Kindly review this and let me know if this implementation works!

//create a gitignore file to ignore the test reports folder
gitIgnorePath := filepath.Join(r.config.Path, ".gitignore")
gitIgnoreData := []byte("./reports/\n")
err = os.WriteFile(gitIgnorePath, gitIgnoreData, 0777)
Copy link
Member

Choose a reason for hiding this comment

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

Why is this only in the record mode? What if the user starts the test mode directly by importing the testcases from somewhere?Also we need to check if the testreport was generated successfully, and only then add it to the gitignore file

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 to a util function and added it to record and test command. We are calling the util function only if there is no error in the previous insertmock call

pkg/util.go Outdated
@@ -77,6 +77,30 @@ func IsTime(stringDate string) bool {
return false
}

func CreateGitIgnore(logger *zap.Logger, filePath string) error {
gitIgnoreData := []byte("./reports/\n")
Copy link
Member

Choose a reason for hiding this comment

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

The path of the reports directory is incorrect

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tree -a
.
├── .gitignore
├── reports
│ ├── test-run-0
│ │ └── test-set-0-report.yaml
│ └── test-run-1
│ └── test-set-0-report.yaml
└── test-set-0
├── mocks.yaml
└── tests
├── test-1.yaml
├── test-2.yaml
└── test-3.yaml

5 directories, 7 files
If this is the folder structure of the keploy folder, could you please explain how is the path to the reports folder incorrect?

pkg/util.go Outdated Show resolved Hide resolved
pkg/util.go Outdated
func CreateGitIgnore(logger *zap.Logger, filePath string) error {
gitIgnoreData := []byte("./reports/\n")
filePath = filePath + "/.gitignore"
gitIgnoreFIle, err := os.OpenFile(filePath, os.O_CREATE|os.O_WRONLY, 0777)
Copy link
Member

Choose a reason for hiding this comment

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

what is the gitignore file exists already?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added check for when gitignore file already exists

@@ -178,6 +178,10 @@ func (r *Recorder) Start(ctx context.Context) error {
r.telemetry.RecordedTestCaseMock(mock.GetKind())
}
}
err := pkg.CreateGitIgnore(r.logger, r.config.Path)
Copy link
Member

Choose a reason for hiding this comment

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

Why do you need to create the gitignore file during the record mode? We only need to create it when reports are created.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apologies for the confusion with your previous comment

Why is this only in the record mode? What if the user starts the test mode directly by importing the testcases from somewhere?Also we need to check if the testreport was generated successfully, and only then add it to the gitignore file

Removed it from the record command

pkg/util.go Outdated
@@ -77,6 +77,30 @@ func IsTime(stringDate string) bool {
return false
}

func CreateGitIgnore(logger *zap.Logger, filePath string) error {
Copy link
Member

Choose a reason for hiding this comment

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

I think this is a generic function so should be added to global utils instead of pkg utils.

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 to global utils

@Akash-Singh04
Copy link
Contributor Author

@PranshuSrivastava Your requested changes have been made

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.

[feature]: automatically add test reports folder to gitignore
2 participants