Skip to content

Add --json option to save collector payload to JSON#36

Merged
nprizal merged 2 commits intomainfrom
pie-3513-add-option-to-save-json-from-test-collector
Mar 25, 2025
Merged

Add --json option to save collector payload to JSON#36
nprizal merged 2 commits intomainfrom
pie-3513-add-option-to-save-json-from-test-collector

Conversation

@nprizal
Copy link
Contributor

@nprizal nprizal commented Mar 25, 2025

We’re integrating bktec with pytest, which requires reading the output from pytest. While pytest has native JUnit XML, it lacks certain information required by bktec. Since bktec relies on this collector, it’s more convenient to save the JSON output from this collector, so bktec can read it.

To save json payload to a file, you need to add --json=path option to the pytest command. For example

pytest --json=result.json

@nprizal nprizal requested a review from zhming0 March 25, 2025 03:09
@nprizal nprizal self-assigned this Mar 25, 2025
Copy link
Contributor

@zhming0 zhming0 left a comment

Choose a reason for hiding this comment

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

Looks good to me.

Comment on lines 52 to 56
def save_payload(self, path):
""" Save payload into a json file """
Copy link
Contributor

Choose a reason for hiding this comment

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

[nit] the comment looks like a better method name

Suggested change
def save_payload(self, path):
""" Save payload into a json file """
def save_payload_as_json(self, path):
""" Save payload into a json file """

@nprizal nprizal force-pushed the pie-3513-add-option-to-save-json-from-test-collector branch from 5c59e59 to 195c3d6 Compare March 25, 2025 05:11
@nprizal nprizal force-pushed the pie-3513-add-option-to-save-json-from-test-collector branch from 195c3d6 to 5d1db20 Compare March 25, 2025 05:53
@nprizal nprizal requested a review from zhming0 March 25, 2025 05:53
@nprizal
Copy link
Contributor Author

nprizal commented Mar 25, 2025

@zhming0 I made some changes to this PR to only save test data.
Instead of

{
  "format": "json",
  "run_env": {},
  "data": [{"id": "test-1", ...}, {"id": "test-2", ...}]
}

we save

[{"id": "test-1", ...}, {"id": "test-2", ...}]

Let me know what you think

@zhming0
Copy link
Contributor

zhming0 commented Mar 25, 2025

Looks good with the format change.

Overall, I do have a bit concern around the fact that this looks like a "one-way door"-ish. But this is bound to be a problem with bktec and collectors having independent lifecycle, so I don't see harm in getting this change out.

@nprizal nprizal merged commit f6f070d into main Mar 25, 2025
11 checks passed
@nprizal nprizal deleted the pie-3513-add-option-to-save-json-from-test-collector branch March 25, 2025 06:04
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