Add missing getters for unity task command line options #116
Conversation
ghost
added
| [testCase, useSetter, value] << | ||
| [ | ||
| UnityCommandLineOption.argumentFlags.collect( | ||
| { it -> ["${it}", it.flag] }), | ||
| [true, false], | ||
| ["foobar", ""] | ||
| ].combinations() |
There was a problem hiding this comment.
I really don't like these arrays as it is visually super hard to understand what the values will be. I know that you want to save space and don't repeat yourself.
Also this setup and the test case before could be rolled into one or?
There was a problem hiding this comment.
I believe this approach is safer since anytime we add command line options we automatically catch if the setters/getters are not added for them. It is also much easier to maintain.
I would like to avoid the duplication in the setup for these test cases but I haven't yet figured how to factor it out.
There was a problem hiding this comment.
That said, I do understand your point.
I have added comments to these test cases to explain how they work.
Azurelol
force-pushed
the
add/missing_unity_command_line_getters
branch
from
fccdc5e
to
3da5d33
Compare
| // For each option that is a flag (with a boolean value) | ||
| UnityCommandLineOption.flags.collect( | ||
| { it -> ["${it}", it.flag] }), | ||
| // Test with and without the setter | ||
| [true, false], | ||
| // Test both true and false values |
There was a problem hiding this comment.
Well I think that commends won't make this more readable. Again that is why I prefer the table definitions since it might be more verbose but it is clearer which variable will have which value (in most cases)
The commends here now won't help really to see what you want to achieve with the nested array
Description
Add missing getters for Unity task command line options, and a generated test that will catch if any newer ones don't have them implemented.
resolves #114
Changes