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

Serialize test result #391

Conversation

lachmanfrantisek
Copy link
Member

@lachmanfrantisek lachmanfrantisek commented Feb 11, 2020

  • Make the TestResult serializable.

Fixes #380

@softwarefactory-project-zuul
Copy link
Contributor

softwarefactory-project-zuul bot commented Feb 11, 2020

Build failed.

Copy link
Member

@TomasTomecek TomasTomecek left a comment

Works for me locally

In [4]: json.dumps(TestingFarmResult.failed)
Out[4]: '"failed"'

In [7]: json.dumps(TestResult("a", "b", "asd"))
Out[7]: '{"name": "a", "result": "b", "log_url": "asd"}'

please also add tests - that's why we didn't saw this earlier, because no tests cover this

@softwarefactory-project-zuul
Copy link
Contributor

softwarefactory-project-zuul bot commented Feb 12, 2020

Build failed.

@lachmanfrantisek
Copy link
Member Author

lachmanfrantisek commented Feb 12, 2020

@TomasTomecek I've added some tests. Do you know, where the serialization was triggered?

@softwarefactory-project-zuul
Copy link
Contributor

softwarefactory-project-zuul bot commented Feb 12, 2020

Build failed.

@TomasTomecek
Copy link
Member

TomasTomecek commented Feb 12, 2020

@lachmanfrantisek celery triggered it when trying to save the result - so probably make sure that a task result is json-serializable

@softwarefactory-project-zuul
Copy link
Contributor

softwarefactory-project-zuul bot commented Feb 12, 2020

Build succeeded.

@lachmanfrantisek
Copy link
Member Author

lachmanfrantisek commented Feb 12, 2020

@lachmanfrantisek celery triggered it when trying to save the result - so probably make sure that a task result is json-serializable

It's already serializable:

https://github.com/packit-service/packit-service/blob/6adea3c0d6592f65e2b3ff9f25d6b4f8ab52e666/packit_service/worker/handler.py#L160-L163

Problem was in the TestResult.

Copy link
Member

@TomasTomecek TomasTomecek left a comment

LGTM, nicely done!

Signed-off-by: Frantisek Lachman <flachman@redhat.com>
Signed-off-by: Frantisek Lachman <flachman@redhat.com>
@lachmanfrantisek lachmanfrantisek added the mergeit When set, zuul wil gate and merge the PR. label Feb 12, 2020
@softwarefactory-project-zuul
Copy link
Contributor

softwarefactory-project-zuul bot commented Feb 12, 2020

Build succeeded.

@softwarefactory-project-zuul
Copy link
Contributor

softwarefactory-project-zuul bot commented Feb 12, 2020

Build succeeded (gate pipeline).

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit ddd80bd into packit:master Feb 12, 2020
2 checks passed
@lachmanfrantisek lachmanfrantisek deleted the serialize-test-result branch Feb 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mergeit When set, zuul wil gate and merge the PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TypeError: Object of type TestResult is not JSON serializable
2 participants