Builds.get() new arg: fields

[GeneMyslinsky]

I have a use case where I want to be able to see all running builds without having to iterate over by item.
Seeing the duration and status is also useful.

There are other useful fields as well which I've included in the docstring.

I didn't run the tests. Sorry.

[pbelskiy]

Hi! Thanks for PR, please fix CI errors

[GeneMyslinsky]

I did add 2 additional new optional args to specify a start and stop index, this helps for builds which have over 1000+ runs in the history.

Tox passed for me locally.
Feel free to make any changes.

from the docs:

For array-type properties, a range specifier is supported. For example, tree=jobs[name]{0,10} would retrieve the name of the first 10 jobs. The range specifier has the following variants:

{M,N}: From the M-th element (inclusive) to the N-th element (exclusive).
{M,}: From the M-th element (inclusive) to the end.
{,N}: From the first element (inclusive) to the N-th element (exclusive). The same as {0,N}.
{N}: Just retrieve the N-th element. The same as {N,N+1}. 

[GeneMyslinsky]

don't mind the function name, no way I could figure out how to make the API give me ONLY running jobs

[pbelskiy]

Please add tests for it

[GeneMyslinsky]

@pbelskiy how do you want me to add tests for this?
Your current tests are just just mocking the response that jenkins would give back.

I could add another mock string, but why not just test if the request was formulated properly instead of a mock response?

BUILDS_ALL_JSON_SELECTIVE = """{
    "_class": "hudson.model.FreeStyleProject",
    "allBuilds": [
        {
            "_class": "hudson.model.FreeStyleBuild",
            "number": 2,
            "url": "http://localhost:8080/job/jobbb/2/"
        },
        {
            "_class": "hudson.model.FreeStyleBuild",
            "number": 3,
            "url": "http://localhost:8080/job/jobbb/3/"
        },
        {
            "_class": "hudson.model.FreeStyleBuild",
            "number": 4,
            "url": "http://localhost:8080/job/jobbb/4/"
        }
    ]
}
"""
@responses.activate
def test_get_selective(client):
    responses.add(
        responses.GET,
        re.compile(r'.*/api/json'),
        body=BUILDS_ALL_JSON_SELECTIVE,
    )
    fields = ['number', 'url']
    response = client.builds.get('job', fields=fields, start=2, end=4)
    print(response)
    assert len(response) == 3
    assert response[0]['number'] == 2

I think the real way to go forward is to spin up a docker container and execute the tests against a fresh instance.

let me know your thoughts here.

EDIT: I added them anyways, I still think docker tests are the way to go though.

[pbelskiy]

@GeneMyslinsky yes, you must add tests with mock, to check that URLs are formed correctly.
You can look on examples in tests/* directory.

Using real Jenkins instance and integration tests here is over-engineering and not correct I suppose.

I did it in far past here https://github.com/pbelskiy/aiojenkins but It's not good practice due to a lot of cons than pros.

[GeneMyslinsky]

@GeneMyslinsky yes, you must add tests with mock, to check that URLs are formed correctly. You can look on examples in tests/* directory.

Using real Jenkins instance and integration tests here is over-engineering and not correct I suppose.

I did it in far past here https://github.com/pbelskiy/aiojenkins but It's not good practice due to a lot of cons than pros.

I like that test suite you have in aiojenkins, because it is essentially a library of examples.
Out of curiosity why did you decide against going that route? What were the cons?

[pbelskiy]

it's incorrect, because we are pinned on http://server? Need to check url, please see the examples.

Moreover I suggest you to check all possible parameters

1. With only start
2. With only end
3. With start and end
4. Without it
5. Check that fields forms correctly

[pbelskiy]

Argument description must be on a new line

[pbelskiy]

Thanks for such serious tests, but I think it over complicated.

It's enough just to check few simple tests cases or correctly formed relative URL.

Pinning on http://server... in test is incorrect due to it's additional dependency, for example if I will check if conftest.py http://server to something else, your tests will be failed, so it's more correctly just to check quiery parameters instead of full absolute URL.

[pbelskiy]

@GeneMyslinsky Thanks for PR, lets do little changes and merge it finally.

About using Docker, I don't like it, because it's slow for local and CI development and testing + additional dependencies and so on. I suppose it's enough to check it locally and then add mock.