Feat/parameters

[dgrisham]

Added a feature for inputting parameters to tests, see the commit message for more details. This will make it easier to run tests over various parameters (number of nodes, file size, etc.), and works nicely with the write_to_file feature for saving output to files whose names reflect the parameters they were ran with.

Commit message (note: this is now out-of-date, see actual commit message):

Added a feature that allows tests to be parameterized over specified variables, whose concrete values may be assigned via command line arguments at runtime. Parameters may be specified in tests by %{P}, where P is the parameter identifer (which can be any string without whitespace), or by %{P, V}, where V is the default value for P.

Example: running a test with go run main.go i=1 replaces all occurrences of %{i} and %{i, <default_value>} with 1. If the same test is instead run without the i=1 argument (i.e. go run main.go), then the program will find i's default value by searching the test for a string of the form %{i, <default_value>}. It will then replace all %{i} and %{i, <v>} instances with <default_value>.

The <default_value> for a parameter should only be specified once. So, if i's default value is 0, then the first time i is used should look like %{i, 0}, then all following references to i should just use %{i}.

[dgrisham]

@hsanjuan @ZenGround0 feedback welcome :)

[dgrisham]

@ZenGround0 Hey Wyatt, yeah that's a great point. I actually hadn't thought about that, but you're exactly right. However, now that I think about it, I think it should be okay. As it is, it would allow the user to set a parameter as empty with %{<param>,}, which may be useful in some cases and shouldn't break anything (like you said, it would just match on the empty string). There might be something I'm missing though or a good reason to force it to be one or more characters, but it seems like as long as the user pays attention to what they're doing it should be fine.

[ZenGround0]

Awesome, that makes sense. In that case LGTM.

[dgrisham]

@hsanjuan Hey Hector, I appreciate the thoroughness of your comments, and definitely agree with your points. The current implementation certainly feels tacked-on and would be kind of a pain to extend in the future (e.g. if we wanted to add additional command line flags, like you said). I'll start tackling each of your points to make a cleaner implementation of this feature :)

[dgrisham]

@hsanjuan @ZenGround0 Hey guys, I just pushed an intermediate commit that focused on the CLI flag parsing. I went with the flag package and made sure that the implementation would be straightforward to extend.

Running go run main.go (or go run main.go -h) shows the help/usage message, which looks like:

USAGE
  kubernetes-ipfs [--param=<name>:<value>,...] <testfile>

OPTIONS
  -param <name>:<value>
        Replace all test parameter instances of <name> with <value>.
        Separate multiple <name>:<value> inputs with ',' or pass this flag multiple times.
  -help
        Show this help message and exit

For passing parameters, you can do:

go run main.go --param N:3

Either -param or --param works, thanks to the flag package. For multiple parameters, you can do:

go run main.go --param N:3,M:4

or

go run main.go --param N:3 --param M:4

If the same parameter is specified multiple times, it uses the most recent version.

[dgrisham]

Hey @hsanjuan, just pushed the rest of the changes you requested (except for the file-input for parameters). See the commit message for details on those changes :)

[hsanjuan]

seems you forgot to add that opts.go file :)

[dgrisham]

@hsanjuan Oh, haha whoops, thanks :+1: Just added.

[dgrisham]

@hsanjuan @ZenGround0 requested changes addressed, and the params file feature has been added. As before, see commit message for more details :) (params file info contained at the end of the commit message).

[dgrisham]

@hsanjuan Regarding your test idea:

• 'Check that replacing works in different variables with different forms given a file.

By 'different forms', do you mean something like: ensure that {{ N }} and {{M}} (no spaces) both get replaced correctly?

[hsanjuan]

@dgrisham yeah

[dgrisham]

Hey @hsanjuan, just pushed tests as well.

[hsanjuan]

@FrankPetrilli I'm good with this. Wanna do the honors and merge ?

Thanks!

[hsanjuan]

@dgrisham seems you have to rebase...

[dgrisham]

@hsanjuan gotcha, just did/pushed

[hsanjuan]

@dgrisham github disagrees...

[dgrisham]

@hsanjuan yep, my bad, better now :)

[ZenGround0]

@FrankPetrilli Hey Frank could you possibly check this out and merge if everything looks good? Thank you!