Skip to content
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

Remove --quite flag and extract cloud test run URL #2

Merged
merged 3 commits into from
Apr 19, 2024

Conversation

DefCon-007
Copy link
Collaborator

Ticket - https://github.com/grafana/k6-cloud/issues/2234

This is a pre-requisite for adding functionality for commenting a link to the cloud test run URL.

Here, we remove the --quite flag from the run command and parse the k6 command output to extract the cloud run URL. We also ignore the test progress output to prevent the shell from getting polluted with multiple log lines of progress updates.

@DefCon-007 DefCon-007 changed the title Remove --quite flag and extract cloud test run URL/ Remove --quite flag and extract cloud test run URL Apr 15, 2024
@DefCon-007 DefCon-007 requested a review from Lantero April 15, 2024 14:11
- Ignore progress updates from stdout
- Extract cloud test run urls from k6 command output
@DefCon-007
Copy link
Collaborator Author

Even without the --quite flag the output is clean like below
Screenshot 2024-04-15 at 17 09 26

Copy link

@Lantero Lantero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whilst I don't want to dismiss the PR, I would prefer the raw output parsing to be a temporary solution, and something that would eventually be replaced in favour of a more robust/powerful solution (e.g. a Github app that can query our backend API's not just for the test run url, but any other test run data, using stable API's, e.g. using a structured output format).

Generally speaking, parsing a non-structured output (Doing things like trimming ASCII art and all that) feels fragile and prone to break across different k6 versions, if OSS team decides to tweak how it looks.

I don't want to block the PR btw, so this is mostly a concern about non-structured output parsing being part of the final solution and the implications of that in the long term. I wonder if k6 json output could be used instead for building an "opinionated" CLI output, if that's the aim here, since structured output will likely be much more stable and less error prone, like some kind of API contract. 🤔

@DefCon-007
Copy link
Collaborator Author

I would prefer the raw output parsing to be a temporary solution

Me too, this currently is quite fragile but we don't have any other option to easily get the test run URL and not mess up the run action output in the pipeline. It would have been helpful if the removal of quite flag didn't remove the cloud url from the output too.

I wonder if k6 json output could be used instead for building an "opinionated" CLI output

The JSON output does not have the cloud test run url link.

The only thing we are trying to achieve here is prevent outputting the test progress on stdout and extract cloud test run URL.

@Lantero
Copy link

Lantero commented Apr 18, 2024

I would prefer the raw output parsing to be a temporary solution

Me too, this currently is quite fragile but we don't have any other option to easily get the test run URL and not mess up the run action output in the pipeline. It would have been helpful if the removal of quite flag didn't remove the cloud url from the output too.

I wonder if k6 json output could be used instead for building an "opinionated" CLI output

The JSON output does not have the cloud test run url link.

The only thing we are trying to achieve here is prevent outputting the test progress on stdout and extract cloud test run URL.

Makes sense. It's fine by me as long as we have plans to improve it in the future 👍

@DefCon-007 DefCon-007 merged commit eb44641 into main Apr 19, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants