1177531 - Add tests for various get functions in treeherder-client

Status: RESOLVED FIXED

(Tree Management Graveyard :: Treeherder: Client Libraries, defect, P3)

Description

[Vaibhav (:vaibhav1994)]

We need more tests for functions (like get_jobs, get_resultsets etc.) in treeherder-client. Lets use this bug to track it.

Comment 1

[MikeLing]

Hi~ I would like to work on this issue, could you give me some examples about this? Thank you:)

Comment 2

[Vaibhav (:vaibhav1994)]

Mike, I was at the work-week and somehow missed this.
You can check the tests for client.py here: https://github.com/mozilla/treeherder/blob/master/tests/client/test_treeherder_client.py

Comment 3

[MikeLing]

(In reply to Vaibhav (:vaibhav1994) from comment #2)
> Mike, I was at the work-week and somehow missed this.
> You can check the tests for client.py here:
> https://github.com/mozilla/treeherder/blob/master/tests/client/
> test_treeherder_client.py

Hm, so the things I need to do is add some tests for get_jobs and get_resultsets inside TreeherderClientTest class which is  https://github.com/mozilla/treeherder/blob/master/tests/client/test_treeherder_client.py#L410. Isn't it?

Comment 4

[MikeLing]

Hi Vaibhav,

Could you brief something about mock_post.call_args (https://github.com/mozilla/treeherder/blob/master/tests/client/test_treeherder_client.py#L438) for me? I'm new to treeherder python client, so, I'm not about treeherder.client.client.requests.post.call_args' meaning.

Thank you:)

Comment 5

[Vaibhav (:vaibhav1994)]

@patch decorator makes it easy to mock classes/functions in a file. So it is mocking the post function in treeherder-client, since we do not actually want to post the test data.

Comment 6

[MikeLing]

Attachment: PR for bug 1177531

Sorry for the late respond. I'm not sure whether I do the right way to test for it. So I decide to ask feedback for now, please give me some advice on it. Thank you very much!

Comment 7

[Vaibhav (:vaibhav1994)]

Comment on attachment 8651034 [details] [review]
PR for bug 1177531

This does not look like the right way. You will need to make a mock get function for a request, and compare that result to a pre-known outcome.
There are various example for testing get functions lying in the codebase like: https://github.com/MikeLing/treeherder/blob/bugfix-1177531/tests/client/test_perfherder_client.py#L10

I think :wlach will give you a more detailed feedback, since he has more experience with the codebase.

Comment 8

[MikeLing]

(In reply to Vaibhav (:vaibhav1994) from comment #7)

Cool, I know what should I do this time. Could you or :wlach tell me which api I should wrote test for them and show me the usage of them? Something like https://treeherder.mozilla.org/api/project/mozilla-inbound/performance-data/get_performance_series_summary/?interval=86400, so I can understand what's the json data looks like and how to mock them. 

Thank you Vaibhav, great appreciated :)

Comment 9

[Vaibhav (:vaibhav1994)]

Ok, so for get_jobs method in Treeherder-client[1], the url goes like this[2]. Similarly for get_resultset method, you can construct the url from the method parameters. A good documentation for these urls is given in treeherder docs[3].

[1] - https://github.com/mozilla/treeherder/blob/master/treeherder/client/thclient/client.py#L811
[2] - https://treeherder.mozilla.org/api/project/mozilla-inbound/jobs/?result_set_id=15558
[3] - https://treeherder.mozilla.org/docs/#!/project/Result_Set_list

Comment 10

[Vaibhav (:vaibhav1994)]

Comment on attachment 8651034 [details] [review]
PR for bug 1177531

William might be a better reviewer for this :)

Left a comment in the PR.

Comment 11

[William Lachance (:wlach)]

Comment on attachment 8651034 [details] [review]
PR for bug 1177531

I like where this is going but would like to see some changes before it goes in.

Comment 12

[Treeherder GitHub Bugbot]

Commits pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/6af23d435679cec1a672a01635dd2f888b76e312
Bug 1177531 - Add tests for various get functions in treeherder-client

https://github.com/mozilla/treeherder/commit/1b4c8aeb5a59faf58d0a3f5ca5366717325ae1d2
Merge pull request #894 from MikeLing/bugfix-1177531

Bug 1177531 - Add tests for various get functions in treeherder-client

Comment 13

[William Lachance (:wlach)]

Comment on attachment 8651034 [details] [review]
PR for bug 1177531

This is good. Merged. There are still quite a few other methods to test, so leaving this bug open for now. :)

Comment 14

[MikeLing]

(In reply to William Lachance (:wlach) from comment #13)
> Comment on attachment 8651034 [details] [review]
> PR for bug 1177531
> 
> This is good. Merged. There are still quite a few other methods to test, so
> leaving this bug open for now. :)

ok, please tell me if you found any function need test, and i would happy to work on that :)

Comment 15

[Ed Morley [:emorley]]

Some tests did already land, we can always open another bug for more tests.