Should be in stderr. Please check analogous cases
Can we do something with the magic number 4 in the comments as well? Here and elsewhere
Generally LGTM, but errors should go to stderr. Ideally, we should also change 1 2 3 4 to use a constant (something like 1 ... TEST_GROUPS). Please fix these minor issues and I can merge it ASAP.
Can we repeat order of the variables for readability? I.e. run_valgrind and then extra_flags to match order above
Is it required? It needs to be updated each time the set variables of the method are updated, yet it does not provide additional data. It would make more sense to explain what each variable means next to their initiation place.
It shouldn't be at the stderr. It would make more sense to put it in the stdout, since it's informational/debug data.
It shouldn't be at the stderr. It would make more sense to put it in the stdout, since it's informational/debug data.
50 ms clarifying comment was on point. But it should be formatted in the same manner as other comments (for inline comment, use either /* or //, not both)
Why 2 builds (0 and 1) share the same ports range?
Thank you for the PR. It's a great functional improvement. On the other note, I'd like to see slightly better practices in terms of readability and general coding best practices (avoiding magic numbers, commentary that add context rather than repeat code, and, importantly, unified error handling stategy). Nonetheless, solid PR overall.
Wouldn't it create trash in non-CI (local) run?