-
-
Notifications
You must be signed in to change notification settings - Fork 377
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
feat: autogenerate config if it doesn't already exist at the path #1773
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Pranshu Srivastava <iampranshu24@gmail.com>
Signed-off-by: Pranshu Srivastava <iampranshu24@gmail.com>
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.
FIx the github actions
Signed-off-by: Pranshu Srivastava <iampranshu24@gmail.com>
Signed-off-by: Pranshu Srivastava <iampranshu24@gmail.com>
Signed-off-by: Pranshu Srivastava <iampranshu24@gmail.com>
…o autogen-config
Signed-off-by: Pranshu Srivastava <iampranshu24@gmail.com>
5113cd4
to
cee0035
Compare
Signed-off-by: Pranshu Srivastava <iampranshu24@gmail.com>
Signed-off-by: Pranshu Srivastava <iampranshu24@gmail.com>
Signed-off-by: Pranshu Srivastava <iampranshu24@gmail.com>
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.
Please address the comments and resolve the pipeline failures
@@ -69,9 +69,6 @@ sudo -E env PATH=$PATH ./../../keployv2 test -c 'npm start' --delay 10 --generat | |||
|
|||
sudo -E env PATH=$PATH ./../../keployv2 test -c 'npm start' --delay 10 --testsets test-set-0 --generateGithubActions=false | |||
|
|||
# Generate the keploy-config file. | |||
sudo ./../../keployv2 config --generate | |||
|
|||
# Update the global noise to ts. | |||
config_file="./keploy.yml" |
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.
how will these filters appended to the config if you are not generating them ahead..?
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.
in our case the config file would have already been generated when we are running keploy in the record and the test mode. So this command is not needed now, we can just edit the same keploy config file.
cli/provider/cmd.go
Outdated
@@ -294,7 +294,7 @@ func (c *CmdConfigurator) ValidateFlags(ctx context.Context, cmd *cobra.Command) | |||
utils.LogError(c.logger, err, errMsg) | |||
return errors.New(errMsg) | |||
} | |||
c.logger.Info("config file not found; proceeding with flags only") | |||
c.logger.Info("config file not found; creating one and proceeding with flags for now.") |
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.
remove "for now" from the sentence. and mention that you are creating config file from the flag that are used.
cli/provider/service.go
Outdated
@@ -83,7 +85,21 @@ func (n *ServiceProvider) GetService(ctx context.Context, cmd string) (interface | |||
case "config", "update": | |||
return tools.NewTools(n.logger, tel), nil | |||
// TODO: add case for mock | |||
case "record", "test", "mock", "normalize": | |||
case "record", "test", "mock": |
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 is the normalize removed from switch case..?
cli/provider/service.go
Outdated
// Check if the config file exists on the path or not and if it does not, we create it. | ||
if !utils.CheckFileExists("keploy.yml") { | ||
toolsService := tools.NewTools(n.logger, tel) | ||
n.cfg.Path = strings.TrimSuffix(n.cfg.Path, "/keploy") |
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.
use a local variable rather an trimming and attaching to config object..?
if err != nil { | ||
n.logger.Debug("failed to marshal the config") | ||
} | ||
err = toolsService.CreateConfig(ctx, "keploy.yml", string(yamlData)) |
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.
Can you provide a screenshot by using 3 flags like selected test-sets etc and generate the config file..?
Signed-off-by: Pranshu Srivastava <iampranshu24@gmail.com>
Signed-off-by: Pranshu Srivastava <iampranshu24@gmail.com>
@PranshuSrivastava , Is this ready for merge ? as we need this in VsCode Extension. |
Signed-off-by: Pranshu Srivastava <iampranshu24@gmail.com>
Signed-off-by: Pranshu Srivastava <iampranshu24@gmail.com>
Related Issue
It would be good to autogenerate a config file for the user if it doesn't already exist
Closes: #1737
Describe the changes you've made
Checking if the config file exists or not, and if it doesn't create one with the flags and parameters provided in the command
Type of change
Checklist: