[Libosinfo] [PATCH osinfo-db 7/7] tests: Add custom pytest-3 --network-tests option

[Cole Robinson]

On 3/21/19 11:21 AM, Fabiano Fidêncio wrote:
> On Thu, Mar 21, 2019 at 4:07 PM Cole Robinson <crobinso at redhat.com> wrote:
>>
>> On 3/21/19 8:23 AM, Fabiano Fidêncio wrote:
>>> On Wed, 2019-03-20 at 17:53 -0400, Cole Robinson wrote:
>>>> This wires up a --network-tests option to save us from needing the
>>>> OSINFO_DB_NETWORK_TESTS environment variable, though that method
>>>> still
>>>> works. It works a bit differently in that unless specified it
>>>> entirely
>>>> takes the tests out of the pool and doesn't list any SKIP options.
>>>>
>>>> IMO this is nicer because it makes the default test suite output less
>>>> noisy. The network tests need to only be run periodically
>>>>
>>>> Signed-off-by: Cole Robinson <crobinso at redhat.com>
>>>> ---
>>>>  tests/conftest.py  | 17 +++++++++++++++++
>>>>  tests/test_urls.py |  9 ---------
>>>>  2 files changed, 17 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/tests/conftest.py b/tests/conftest.py
>>>> index 8bd7236..5f9cd5b 100644
>>>> --- a/tests/conftest.py
>>>> +++ b/tests/conftest.py
>>>> @@ -13,6 +13,23 @@ def _setup_env():
>>>>
>>>>  _setup_env()
>>>>
>>>> +
>>>> +def pytest_addoption(parser):
>>>> +    parser.addoption("--network-tests", action="store_true",
>>>> default=False,
>>>> +            help=("Run osinfo-db network tests. Same tests as
>>>> triggered "
>>>> +                  "by setting env variable
>>>> OSINFO_DB_NETWORK_TESTS"))
>>>> +
>>>> +
>>>> +def pytest_ignore_collect(path, config):
>>>> +    """
>>>> +    Entirely skip loading test_urls.py if the option wasn't
>>>> specified
>>>> +    """
>>>> +    run_network = bool(config.getoption("--network-tests") or
>>>> +            os.environ.get("OSINFO_DB_NETWORK_TESTS"))
>>>> +    if os.path.basename(str(path)) == "test_urls.py" and not
>>>> run_network:
>>>> +        return True
>>>> +
>>>> +
>>>>  # This will trigger some DATA_DIR validation
>>>>  from . import util
>>>>  dummy = util
>>>> diff --git a/tests/test_urls.py b/tests/test_urls.py
>>>> index 552e6eb..5900f3d 100644
>>>> --- a/tests/test_urls.py
>>>> +++ b/tests/test_urls.py
>>>> @@ -1,15 +1,10 @@
>>>>  # This work is licensed under the GNU GPLv2 or later.
>>>>  # See the COPYING file in the top-level directory.
>>>>
>>>> -import os
>>>> -import pytest
>>>> -
>>>>  from . import util
>>>>
>>>>
>>>>  @util.os_parametrize('_os', filter_images=True)
>>>> - at pytest.mark.skipif(os.environ.get('OSINFO_DB_NETWORK_TESTS') is
>>>> None,
>>>> -                    reason='Network related tests are not enabled')
>>>>  def test_images_url(_os):
>>>>      broken = []
>>>>      for image in _os.images:
>>>> @@ -20,8 +15,6 @@ def test_images_url(_os):
>>>>
>>>>
>>>>  @util.os_parametrize('_os', filter_trees=True)
>>>> - at pytest.mark.skipif(os.environ.get('OSINFO_DB_NETWORK_TESTS') is
>>>> None,
>>>> -                    reason='Network related tests are not enabled')
>>>>  def test_medias_url(_os):
>>>>      broken = []
>>>>      for media in _os.medias:
>>>> @@ -32,8 +25,6 @@ def test_medias_url(_os):
>>>>
>>>>
>>>>  @util.os_parametrize('_os', filter_media=True)
>>>> - at pytest.mark.skipif(os.environ.get('OSINFO_DB_NETWORK_TESTS') is
>>>> None,
>>>> -                    reason='Network related tests are not enabled')
>>>>  def test_trees_url(_os):
>>>>      broken = []
>>>>      for tree in _os.trees:
>>>
>>> Cole,
>>>
>>> This one didn't work for me at all. I've tried:
>>> - `make check`
>>> - `pytest3 tests/test_urls.py`
>>>
>>> In both cases the URL tests were not skipped.
>>>
>>> Is there some setup needed on my environment?
>>>
>>
>> Ah I didn't test 'make check', sorry, it is indeed busted. I can fix it
>> easy enough, but...
> 
> Mind that I've failed to have the tests skipped even running a simple:
> `pytest-3 tests/test_urls.py`
> Seems that the methods added are never ever called.
> 

As you mentioned elsewhere, just invoking 'pytest-3' does the right
thing: plain invocation skips the tests, but calling with
--network-scripts option or the env variable set invokes the tests.
That's what I was testing with

The conftest.py bits alter how pytest finds/collects test files. If a
test file isn't explicitly specified, that code will kick in, and skip
test_urls unless the option or env var is specified.

pytest-3 tests/test_urls.py is an explicit request to run that file, so
it skips the file collection logic entirely. That's why the tests are
always run in your case. Whether that's a bug or a feature is debatable:
IMO if a user explicitly requests 'hey run test_urls.py' it shouldn't
also require a special environment variable to actually run, which is
the current state. It's a minor point though

>>
>> By having 'make check' run each test file individually with pytest-3 we
>> lose some of the flexibility of pytest. I know you implemented it that
>> way to have individual test file logfiles, which doesn't seem to have
>> first class support in pytest
>>
>> Is the multiple log files specifically something you like, or were you
>> just aiming to reproduce standard 'make check' behavior? Would just one
>> log file suit your needs?
> 
> I was just trying to mimic`make check` behaviour. I don't have any
> strong feelings for changing it to whatever is more natural with
> regards to using pytest. So, go ahead. :-)
> 

Okay I'll play with it and send a v2 including some version of this patch

Thanks,
Cole