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 commented Feb 11, 2020

  • Make the TestResult serializable.

Fixes #380

@softwarefactory-project-zuul

This comment has been minimized.

Copy link
Contributor

softwarefactory-project-zuul bot commented Feb 11, 2020

Build failed.

Copy link
Contributor

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

@lachmanfrantisek lachmanfrantisek force-pushed the lachmanfrantisek:serialize-test-result branch from a178ee1 to 0a42e71 Feb 12, 2020
@softwarefactory-project-zuul

This comment has been minimized.

Copy link
Contributor

softwarefactory-project-zuul bot commented Feb 12, 2020

Build failed.

@lachmanfrantisek

This comment has been minimized.

Copy link
Member Author

lachmanfrantisek commented Feb 12, 2020

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

@lachmanfrantisek lachmanfrantisek force-pushed the lachmanfrantisek:serialize-test-result branch from 0a42e71 to 3af8512 Feb 12, 2020
@softwarefactory-project-zuul

This comment has been minimized.

Copy link
Contributor

softwarefactory-project-zuul bot commented Feb 12, 2020

Build failed.

@TomasTomecek

This comment has been minimized.

Copy link
Contributor

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

@lachmanfrantisek lachmanfrantisek force-pushed the lachmanfrantisek:serialize-test-result branch from 3af8512 to 40e1723 Feb 12, 2020
@softwarefactory-project-zuul

This comment has been minimized.

Copy link
Contributor

softwarefactory-project-zuul bot commented Feb 12, 2020

Build succeeded.

@lachmanfrantisek

This comment has been minimized.

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:

class HandlerResults(dict):
"""
Job handler results.
Inherit from dict to be JSON serializable.

Problem was in the TestResult.

Copy link
Contributor

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 force-pushed the lachmanfrantisek:serialize-test-result branch from 40e1723 to c5ec271 Feb 12, 2020
@softwarefactory-project-zuul

This comment has been minimized.

Copy link
Contributor

softwarefactory-project-zuul bot commented Feb 12, 2020

Build succeeded.

@softwarefactory-project-zuul

This comment has been minimized.

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-service:master Feb 12, 2020
3 checks passed
3 checks passed
LGTM analysis: Python No new or fixed alerts
Details
local/check check status: success
Details
local/gate gate status: success
Details
@lachmanfrantisek lachmanfrantisek deleted the lachmanfrantisek: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
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants
You can’t perform that action at this time.