adding support for custom templates #22

shubhamarora · 2020-10-09T21:29:39Z

What's changing

*Changes to present POC of impl. More changes are yet to be done.

Change request: #6

Adding support for user to provide their own custom templates. After this change user will be able to provide path/to/custom/template and extension via parameters when running replay-zero.

Sample command: replay-zero --output=/path/to/templates/gatling_default.template --extension=scala

What else might be impacted?

...

Checklist

[ ] Unit tests (updated and/or added)
[ ] Documentation (function/class docs, comments, etc.)

Celeo · 2020-10-09T23:57:58Z

Hi, thanks for the PR! We'll have a review over it soon.

Celeo · 2020-10-12T04:58:47Z

templates/gatling_default.template

@@ -0,0 +1,66 @@
+package com.yourcompany.perf.yourservice


@wtait1 Do we want to continue to ship with the gatling template? We already have it, so may as well keep it?

@Celeo My suggestion to this is, that we should stop shipping .go files as default supported template and convert them to .template. I did this to demonstrate the same. And a user can have more templates like this. Let me know if i make sense.

@shubhamarora yes I think moving to .template files is great! In fact, what I'm thinking is that the Karate template should be moved over now as well. Then all test generation is being driven by a .template file, but the "default" formats use .template files that are shipped with the binary, and custom templates are provided via the new flag.

For packaging the default templates into the binary, there are several tools that do this. pkger seems like a good choice (go-bindata is no longer supported, and pkger is intended as a replacement for packr).
https://github.com/markbates/pkger

I've never personally worked with pkger before, but I'd be happy to help out / provide guidance as I think creating this common .template-based approach would be slick

@shubhamarora done

Celeo · 2020-10-12T05:01:01Z

main.go

+
+	// check if template exist at the provided path
+	// TODO: add validation for correctness of template
+	if flags.output != "karate" {


If we are going to support just karate as a built-in, then this should be okay. If we look to support more, though, we'll probably want to hoist those template names into a global variable so we can use it here and in the flag description.

I keep that open to you and @wtait1 to decide the default supported template. My idea is to go ahead with karate as default, ship it with package and for rest, user can pass path to template.

I say we should keep both for now - this task is more about changing how templates are provided to the test generation process. See my longer comment below for more detail on how I think we should ship the default templates

shubhamarora · 2020-10-13T18:42:49Z

@wtait1 We have some decision awaiting, can you please share your views and I can take this change to fruition. Thanks! :)

wtait1

Great work so far Shubham. I think there's a bit more discussion we can have to unify the method for providing templates. See detail in the comments

wtait1 · 2020-10-13T18:49:51Z

main.go

@@ -84,7 +85,8 @@ func readFlags() {
 	flag.IntVarP(&flags.defaultTargetPort, "target-port", "t", 8080, "The port the Replay Zero proxy will forward to on localhost")
 	flag.BoolVar(&flags.debug, "debug", false, "Set logging to also print debug messages")
 	flag.IntVarP(&flags.batchSize, "batch-size", "b", 1, "Buffer events before writing out to a file")
-	flag.StringVarP(&flags.output, "output", "o", "karate", "Either [karate] or [gatling]")
+	flag.StringVarP(&flags.output, "output", "o", "karate", "Either [karate] or [path/to/custom/template]")


Should the name of this flag be changed? Not that it was perfect before, but I would be confused as an end user if a flag named output asked for a file as input 🙂

wtait1 · 2020-10-13T19:14:57Z

templates/gatling_default.template

@@ -0,0 +1,66 @@
+package com.yourcompany.perf.yourservice


@shubhamarora yes I think moving to .template files is great! In fact, what I'm thinking is that the Karate template should be moved over now as well. Then all test generation is being driven by a .template file, but the "default" formats use .template files that are shipped with the binary, and custom templates are provided via the new flag.

For packaging the default templates into the binary, there are several tools that do this. pkger seems like a good choice (go-bindata is no longer supported, and pkger is intended as a replacement for packr).
https://github.com/markbates/pkger

I've never personally worked with pkger before, but I'd be happy to help out / provide guidance as I think creating this common .template-based approach would be slick

wtait1 · 2020-10-13T19:16:43Z

main.go

+
+	// check if template exist at the provided path
+	// TODO: add validation for correctness of template
+	if flags.output != "karate" {


I say we should keep both for now - this task is more about changing how templates are provided to the test generation process. See my longer comment below for more detail on how I think we should ship the default templates

shubhamarora · 2020-10-17T21:07:06Z

main.go

@@ -81,10 +82,11 @@ func readFlags() {
 	}
 	flag.BoolVarP(&flags.version, "version", "V", false, "Print version info and exit")
 	flag.IntVarP(&flags.listenPort, "listen-port", "l", 9000, "The port the Replay Zero proxy will listen on")
-	flag.IntVarP(&flags.defaultTargetPort, "target-port", "t", 8080, "The port the Replay Zero proxy will forward to on localhost")
+	flag.IntVarP(&flags.defaultTargetPort, "target-port", "p", 8080, "The port the Replay Zero proxy will forward to on localhost")


Need to update doc about this, if this is mentioned somewhere

See latest review comment for more on docs

shubhamarora · 2020-10-17T21:07:59Z

main.go

+	flag.StringVarP(&flags.template, "template", "t", "karate", "Either [karate] or [gatling] or [path/to/custom/template]")
+	flag.StringVarP(&flags.extension, "extension", "e", "", "For custom output template")


I will update documentation and add details about them after change is finalised

See latest review comment for more on docs

shubhamarora · 2020-10-17T21:10:04Z

@wtait1 @Celeo I think we should not commit vendor folder. go.mod will take care. This will decrease repo side. What do you think?

Celeo · 2020-10-19T19:22:02Z

I believe committing the vendor folder was a decision made when the project was still internal-only. We can revisit it now if there's a notable benefit.

wtait1

@shubhamarora this is shaping up very well, we're almost there!

RE: documentation, we do try to make the tool pretty self-describing via good -h output 🙂 but we do also have a section in the README on different output formats. If you want to add a sub-section there on passing custom templates, that would be awesome.
RE: the vendor folder, yes it would be nice to get rid of that but as Matt said, that's an artifact from our previous development process. I'm inclined to say don't worry about it for now.

wtait1 · 2020-10-21T07:15:03Z

main.go

@@ -81,10 +82,11 @@ func readFlags() {
 	}
 	flag.BoolVarP(&flags.version, "version", "V", false, "Print version info and exit")
 	flag.IntVarP(&flags.listenPort, "listen-port", "l", 9000, "The port the Replay Zero proxy will listen on")
-	flag.IntVarP(&flags.defaultTargetPort, "target-port", "t", 8080, "The port the Replay Zero proxy will forward to on localhost")
+	flag.IntVarP(&flags.defaultTargetPort, "target-port", "p", 8080, "The port the Replay Zero proxy will forward to on localhost")


See latest review comment for more on docs

wtait1 · 2020-10-21T07:15:21Z

main.go

+	flag.StringVarP(&flags.template, "template", "t", "karate", "Either [karate] or [gatling] or [path/to/custom/template]")
+	flag.StringVarP(&flags.extension, "extension", "e", "", "For custom output template")


See latest review comment for more on docs

wtait1 · 2020-10-22T05:45:15Z

README.md

+replay-zero --template=gatling
+```
+
+You can also pass path to your own custom template (in case you dont want to use karate or gatling) to the same paramater and `--extension` or `-e` to pass extentsion of the output.


I give this PR a green light after 2 quick additions

Can we mention here that the templates are following the format from the text/template package in the Go stdlib? So readers have more guidance on where to start

While we're at it, let's also let them know that we support all template functions provided by Sprig (ex. "You can even extend your template's functionality with string manipulation, math operators, and more from the Sprig library"), so they don't end up re-inventing like the wheel like I originally did when this project was started!

wtait1

LGTM. Loved the collaboration / conversation in this PR to bring a great extension to Replay Zero's functionality!

Celeo

Fantastic work! We really appreciate your contribution!

wtait1 · 2020-10-26T03:12:40Z

🚀 PR was released in v0.1.0 🚀

adding support for custom templates

0495b17

shubhamarora requested a review from wtait1 as a code owner October 9, 2020 21:29

Celeo reviewed Oct 12, 2020

View reviewed changes

wtait1 reviewed Oct 13, 2020

View reviewed changes

embedding templates and consuming them

94f5ca8

shubhamarora commented Oct 17, 2020

View reviewed changes

shubhamarora requested review from Celeo and wtait1 October 19, 2020 18:08

wtait1 reviewed Oct 21, 2020

View reviewed changes

updating the readme

1fddc0f

wtait1 reviewed Oct 22, 2020

View reviewed changes

updating the readme

99b3666

wtait1 approved these changes Oct 23, 2020

View reviewed changes

wtait1 added the minor Increment the minor version when merged label Oct 23, 2020

Celeo approved these changes Oct 24, 2020

View reviewed changes

wtait1 merged commit dd8761d into intuit-archive:master Oct 26, 2020

wtait1 added the released This issue/pull request has been released. label Oct 26, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

adding support for custom templates #22

adding support for custom templates #22

shubhamarora commented Oct 9, 2020

Celeo commented Oct 9, 2020

Celeo Oct 12, 2020

shubhamarora Oct 12, 2020

wtait1 Oct 13, 2020

shubhamarora Oct 17, 2020

Celeo Oct 12, 2020

shubhamarora Oct 12, 2020

wtait1 Oct 13, 2020

shubhamarora commented Oct 13, 2020 •

edited

Loading

wtait1 left a comment

wtait1 Oct 13, 2020

wtait1 Oct 13, 2020

wtait1 Oct 13, 2020

shubhamarora Oct 17, 2020

wtait1 Oct 21, 2020

shubhamarora Oct 17, 2020

wtait1 Oct 21, 2020

shubhamarora commented Oct 17, 2020

Celeo commented Oct 19, 2020

wtait1 left a comment

wtait1 Oct 21, 2020

wtait1 Oct 21, 2020

wtait1 Oct 22, 2020

shubhamarora Oct 23, 2020

wtait1 left a comment

Celeo left a comment

wtait1 commented Oct 26, 2020

		flag.StringVarP(&flags.template, "template", "t", "karate", "Either [karate] or [gatling] or [path/to/custom/template]")
		flag.StringVarP(&flags.extension, "extension", "e", "", "For custom output template")

adding support for custom templates #22

adding support for custom templates #22

Conversation

shubhamarora commented Oct 9, 2020

What's changing

What else might be impacted?

Checklist

Celeo commented Oct 9, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shubhamarora commented Oct 13, 2020 • edited Loading

wtait1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shubhamarora commented Oct 17, 2020

Celeo commented Oct 19, 2020

wtait1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wtait1 left a comment

Choose a reason for hiding this comment

Celeo left a comment

Choose a reason for hiding this comment

wtait1 commented Oct 26, 2020

shubhamarora commented Oct 13, 2020 •

edited

Loading