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

Unify run_id header in outputs. #109

Merged
merged 10 commits into from
Jan 16, 2024
Merged

Unify run_id header in outputs. #109

merged 10 commits into from
Jan 16, 2024

Conversation

tomalec
Copy link
Member

@tomalec tomalec commented Jan 11, 2024

Unify header used for run id in --wait and regular mode.
So it's easier to interpret the results.

Before

Currently, a call with --wait returns, but calling it without calls is the same thing as "Test ID".

$ qit run:activation google-listings-and-ads

Running test...
Test started on the QIT Servers!
Test ID    4547685                                                                                                                              
…

$ qit run:activation google-listings-and-ads --wait

Running test...
Test run completed.
Run Id               4547687                                                                                                                              
…

$ qit run:activation google-listings-and-ads --wait --json
{"run_id":4547688,…

After

Both return

Run Id

Alternatives to consider.

qit get call it even more verbosely "test run id", so maybe we should stick to that.

qit get --help
Description:
  Get a single test run.

Usage:
  get [options] [--] <test_run_id>

Arguments:
  test_run_id   

Context

I'm trying to add a bit more GHactions-based automation to interpret the output and pass it to the following actions and workflows.
Having only one format to parse would make it a bit easier.

Also, it would be nice if I could call it w/o --wait but with --json and get the test run ID value.

@tomalec tomalec requested a review from Luc45 January 11, 2024 21:07
@Luc45
Copy link
Member

Luc45 commented Jan 12, 2024

howdy @tomalec, thanks for the feedback.

I agree we could normalize Test ID, Run ID into Test Run ID, both in the text output and in arguments where applicable.

However, any programmatic integration you do based on the output should ideally be done with the --json flag. The non-JSON output is considered just text oriented for the user and is more likely to be changed, whereas the JSON output is more likely to remain unchanged, which makes any programmatic integration you build on top of it more solid.

You can use jq to parse the JSON output that you receive back, eg:

  1. Running a security test and getting the result ID: qit run:security some-extension --json --wait | jq '.run_id' (Returns 123456)
  2. Getting the status: qit get 123456 --json | jq '.status' (Returns "failed")

@tomalec
Copy link
Member Author

tomalec commented Jan 12, 2024

However, any programmatic integration you do based on the output should ideally be done with the --json flag

Thanks for the hint. I'd definitely do that; the problem is quit run:security some-extension --json (without --wait returns, no JSON whatsoever.

I think that's another area for improvement, but it's a bit more complicated for me to do that this PR ;)

@Luc45
Copy link
Member

Luc45 commented Jan 12, 2024

Got it, I'll make it return the async response in that case. PR should be up by EOD.

@Luc45
Copy link
Member

Luc45 commented Jan 12, 2024

@tomalec Can you please pull this branch and see if that solves your use-case?

You can try the development build in this branch with either option:

  • cd this-repo && ./qit run:activation automatewoo --json
  • or cd this-repo/src && php qit-cli.php run:activation automatewoo --json

@tomalec
Copy link
Member Author

tomalec commented Jan 15, 2024

Awesome, thanks! Yes, it does solve my use case

@Luc45
Copy link
Member

Luc45 commented Jan 16, 2024

Now test_run_id is used more consistently, and it's possible to get the JSON response without waiting for the test to finish:

Starting a test run:

image

Getting a test run:

image

Getting a test run JSON (Property test_run_id was added, I kept run_id for backwards compatibility, will be removed later):

image

Running a test with --json flag without waiting for the results:

image

Running a test with --json flag and --wait:

image

@Luc45 Luc45 requested review from a team and removed request for Luc45 January 16, 2024 21:06
@Luc45 Luc45 self-assigned this Jan 16, 2024
Copy link
Contributor

@zhongruige zhongruige left a comment

Choose a reason for hiding this comment

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

Changes look good to me and I was able to kick off test runs from the CLI as expected. :shipit:

@Luc45 Luc45 merged commit d650b65 into trunk Jan 16, 2024
@Luc45 Luc45 deleted the tweak/run_id-header branch January 16, 2024 23:40
@tomalec
Copy link
Member Author

tomalec commented Jan 21, 2024

Thank you for taking over the PR and pushing it so much further!

tomalec added a commit to woocommerce/grow that referenced this pull request Jan 29, 2024
Use changes added in woocommerce/qit-cli#109

Add `ignore_fail` parameter, to simplify continue-on-error behavior.

Drop the report file, use annotations only.

Hide result URLs, expose `test_run_id` for further checks.
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.

3 participants