Code review comment for ppa-dev-tools:results-class

On Wed, Aug 31, 2022 at 06:40:02PM -0000, Bryce Harrington wrote:
>Answer & re-question for one of your feedback items, inline.
>
>Diff comments:
>
>> diff --git a/tests/test_job.py b/tests/test_job.py
>> index d9a9967..44491c2 100644
>> --- a/tests/test_job.py
>> +++ b/tests/test_job.py
>> @@ -71,9 +74,10 @@ def test_request_url():
>> jobinfo = {
>> 'triggers': ['t/1'],
>> 'ppas': ['ppa:a/b', 'ppa:c/d']
>> - }
>> + }
>> job = Job(0, 'a', 'b', 'c', 'd', jobinfo['triggers'], jobinfo['ppas'])
>> - assert job.request_url == "https://autopkgtest.ubuntu.com/request.cgi?release=c&arch=d&package=b&trigger=t/1"
>> + assert job.request_url.startswith("https://autopkgtest.ubuntu.com/request.cgi")
>> + assert job.request_url.endswith("?release=c&arch=d&package=b&trigger=t/1")
>
>Yeah it was for shortening the length for the lint warning. Normally I'd do the new variables but figured that would make the testing less clear.
>
>My thinking here for two tests is that it now checks a) is the cgi address correctly inserted, and then b) are the args showing up properly. Do you think it'd be better to just check the whole URL?
>

This was just a nitpick to point out that the test changed. To keep the
behavior, i.e, for the commit to be a refactoring only commit, it should
keep checking the whole thing. It should be OK to merge it as is though
:)

>>
>>
>> def test_get_running():
>
>
>--
>https://code.launchpad.net/~bryce/ppa-dev-tools/+git/ppa-dev-tools-1/+merge/429187
>You are reviewing the proposed merge of ppa-dev-tools:results-class into ppa-dev-tools:main.
>

--
Athos Ribeiro