Add support for --version flag #3442
Conversation
|
|
cmd/tests/cmd_run_test.go
Outdated
| @@ -42,7 +42,7 @@ func TestVersion(t *testing.T) { | |||
| cmd.ExecuteWithGlobalState(ts.GlobalState) | |||
|
|
|||
| stdout := ts.Stdout.String() | |||
| assert.Contains(t, stdout, "k6 v"+consts.Version) | |||
| assert.Contains(t, stdout, "k6 version "+consts.Version) | |||
There was a problem hiding this comment.
It prints
k6 version 0.47.0 (go1.21.1, linux/amd64)
That means it would be a breaking change for someone relying on it on automated tools. And, it doesn't match really the version we usually announce, check https://github.com/grafana/k6/releases/tag/v0.47.0 for an example.
Can you roll back it, please? I expect you're doing it because it is the default template for Cobra but if you use SetVersionTemplate then you should be able to change it.
There was a problem hiding this comment.
Hi @codebien !
Thank you for your feedback. I did change that and the corresponding test to match Cobra's default.
I'll rollback that and change the default.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #3442 +/- ##
==========================================
- Coverage 73.35% 73.30% -0.05%
==========================================
Files 261 259 -2
Lines 19742 19773 +31
==========================================
+ Hits 14481 14495 +14
- Misses 4367 4376 +9
- Partials 894 902 +8
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
@ffapitalle Thanks for the changes.
I think we're now missing two more things:
Must to have: add to the related test here https://github.com/grafana/k6/blob/e44a08fa5d642ba148c08d4a1eb291de576009c5/cmd/tests/cmd_run_test.go#L37C1-L52C2 the case with the new flag. It requires a bit of refactoring to have a table test. Or a simpler alternative could be to move the test to a new cmd_version_test.go file and have a dedicated test like TestVersionFlag.
Nice to have: having the k6 version command calling the root's command or vice versa. In this way, we are 100% sure that one change will be also applied to the other.
cmd/root.go
Outdated
| } | ||
|
|
||
| rootCmd.SetVersionTemplate( | ||
| `{{with .Name}}{{printf "%s " .}}{{end}}{{printf "v%s" .Version}}\n`, |
There was a problem hiding this comment.
| `{{with .Name}}{{printf "%s " .}}{{end}}{{printf "v%s" .Version}}\n`, | |
| `{{with .Name}}{{printf "%s " .}}{{end}}{{printf "v%s" .Version}}`, |
Is it required? On Alacritty I get the following output:
k6 v0.47.0 (go1.21.1, linux/amd64)\n%
In any case, even after removing the newline the % char still remains. It requires a bit of investigation about why.
There was a problem hiding this comment.
My fault on this. It was the wrong place to include the newline character. I've fixed it.
I went for the table test schema.
I gave it a few shots but with no luck. Both (flag and subcommand) rely on the |
cmd/tests/cmd_run_test.go:49 whitespace unnecessary leading newline
Co-authored-by: Ivan <[email protected]>
olegbespalov
added
ux
documentation-needed
What?
Adds support for
--versionglobal flag that matches theversionsubcommand.Why?
Running the usual Unix-like flags returns an error.
Checklist
make lint) and all checks pass.make tests) and all tests pass.Related PR(s)/Issue(s)
#2352