-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP Add a k6 cloud run --local-execution
mode of execution
#3818
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## cloud_subcommands #3818 +/- ##
====================================================
Coverage ? 70.98%
====================================================
Files ? 293
Lines ? 21827
Branches ? 0
====================================================
Hits ? 15494
Misses ? 5244
Partials ? 1089
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
I don't know how sensible that would be, but as far as I know, using the dot as separator is not very Windows friendly (it requires quoting, see PowerShell/PowerShell#19640), just to be considered when evaluating it. |
// TODO: Figure out a better way to handle the CLI flags | ||
flags.BoolVar(&c.exitOnRunning, "exit-on-running", c.exitOnRunning, | ||
"exits when test reaches the running status") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add some kind of validation for incompatible flags? For instance, --exit-on-running
& --local-execution
, which I think don't have sense together.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great idea, let's look into it 👍🏻
flags.BoolVar(&c.uploadOnly, "upload-only", c.uploadOnly, | ||
"only upload the test to the cloud without actually starting a test run") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #3850
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dropping this 👍
Important notice: this commit declare a cobra sub-command holding the logic for the `k6 cloud run` sub-command, but does not register it. In this commit, we duplicate the logic from the existing `k6 cloud` logic, with very little adjustments, to support the later registry of the `k6 cloud run` command. To simplify the collaboration on this and further reviews, we delegate any refactoring of the original cloud command's logic, to a further commit or Pull Request.
Important notice: this commit declare a cobra sub-command holding the logic for the `k6 cloud login` sub-command, but does not register it. In this commit, we duplicate the logic from the existing `k6 login` logic, with very little adjustments, to support the later registry of the `k6 cloud login` command. To simplify the collaboration on this and further reviews, we delegate any refactoring of the original cloud command's logic, to a further commit or Pull Request. This new `k6 cloud login` command is notably focusing solely on authenticating with the Grafana Cloud k6, and by design does not aim to support InfluxDB authentication.
Co-authored-by: Joan López de la Franca Beltran <[email protected]>
ddce8cb
to
cf7e0b9
Compare
812efc5
to
9957dc9
Compare
9957dc9
to
25dd426
Compare
aad9c76
to
2b0bc1f
Compare
If we already envision the need for it, isn't better to have a dedicated subcommand? I mean something like |
Closing in favor of #3904 |
What?
This PR adds support for a new
--local-execution
flag to thek6 cloud run
command. Using it, users can run their test script or archive on their local machine, while streaming the results to the cloud.It is built upon the latter, and⚠️ cannot be merged before #3813 ⚠️ .
In the context of this PR, the goal is to reproduce the behavior of the
k6 run -o cloud
behavior, and depending on the advancement of internal work, to also add support for adding archive upload. In a future PR, at a later point in time, we will also add support for logs and traces streaming to the cloud but are dependent on internal work to be able to do so.Why?
We aim to integrate the experience of running tests locally, whilst streaming results to the cloud more intuitive, and integrated in the k6 experience. We also aim to take this opportunity to align some of the logic and behavior involved in running tests in the cloud and running tests locally better.
Task list
--local-execution
. One of the ideas we've been having in the k6 v1 UX work stream revolved around the idea of scoping options with a dot separator like in Prometheus:--local.execution
,--local.send-archive
,--local.send-logs
, etc. cc @dgzlopesk6 cloud run --local-execution
, we should make sure theoutputs
field of the terminal output of the command displays a sensible value (like the URL of the job in the grafana cloud app?). cc @dgzlopesk6 cloud run --local-execution
show the same terminal output ask6 run
ork6 cloud run
?Checklist
make lint
) and all checks pass.make tests
) and all tests pass.Related PR(s)/Issue(s)