Clean up pytest markers

[jankrepl]

Atlas-Download-Tools/tox.ini

Lines 70 to 83 in 829206f

	[pytest]
	testpaths = tests
	addopts =
	-m "not todo and not slow and not internet"
	--cov
	--cov-config=tox.ini
	--no-cov-on-fail
	--durations=20
	--verbosity=1
	--color=yes
	markers =
	internet: requires connection to the internet
	slow: mark denoting a test that is too slow
	todo: mark denoting a test that is not written yet

It seems like the todo and slow markers are not used in the code anymore. Do you think we should keep them? Additionally, there are some tests that requires internet and are not marked with the marker internet. See below an example (maybe there are more?).

Atlas-Download-Tools/tests/test_utils.py

Lines 521 to 529 in 829206f

	# @pytest.mark.internet
	class TestCommonQueries:
	"""A set of tests focused on some basic queries in the CommonQueries class."""
	@pytest.mark.parametrize("dataset_id", EXISTING_DATASET_IDS)
	def test_get_reference_space(self, dataset_id):
	"""Test that all testing datasets are from reference space 9."""
	assert CommonQueries.get_reference_space(dataset_id) == 9

Anyway, I am not sure how useful the internet marker is, however, it can help us identify and filter tests that require it.

[Stannislav]

How do we want to deal with slow tests?

But I'd suggest to for sure delete not slow from addopts in tox because this way we always skip these tests.

[Stannislav]

Whoever takes this ticket, would it be possible to add the slow marker to these two tests:

Atlas-Download-Tools/tests/test_utils.py

Lines 71 to 93 in 43e77aa

	def test_decompression_bomb_warning_suppressed(self, tmpdir):
	image_id = 123
	# Prepare a cached image. The decompression bomb warning is triggered
	# at about 90M pixels, so we prepare an image with 100M pixels.
	img = Image.fromarray(np.zeros((10_000, 10_000), dtype=np.uint8))
	img.save(pathlib.Path(tmpdir) / f"{image_id}-0.jpg")
	with pytest.warns(None) as records:
	get_image(image_id, folder=str(tmpdir))
	assert len(records) == 0
	def test_decompression_bomb_error_triggered(self, tmpdir):
	image_id = 123
	# Prepare a cached image. The decompression bomb error is triggered
	# at about 180M pixels, so we prepare an image with 200M pixels.
	img = Image.fromarray(np.zeros((20_000, 10_000), dtype=np.uint8))
	img.save(pathlib.Path(tmpdir) / f"{image_id}-0.jpg")
	with pytest.raises(Image.DecompressionBombError):
	get_image(image_id, folder=str(tmpdir))

They each take 2-3 seconds.

[jankrepl]

How do we want to deal with slow tests?

But I'd suggest to for sure delete not slow from addopts in tox because this way we always skip these tests.

Well, I guess I am open to discussion here. My two cents:

Locally, I always run

pytest

Which would currently exclude slow, internet and todo tests. This exclusion results in running essential tests that are fast. Which I like.

One can always dynamically overwrite the markers

pytest -m ""  # runs everything

IMO this could be done in the CI since I would not mind if it takes a couple of extra seconds.

[Stannislav]

I guess we need a consensus on what we want to run where 😅

• Locally during dev
• In CI on PRs
• In CI on main

I guess at least in one of the three we should run all tests.

[jankrepl]

Anyway, the todo marker is stupid IMO.

[Stannislav]

Maybe we can try and find a pytest config in tox.ini that suits all of us for local runs? Running pytest in the shell would then give the optimal output straight away.

My preference (skip slow and internet tests, durations=5, verbosity=0, less spamming in the stdout):

[pytest]
testpaths = tests
addopts =
    -m "not slow and not internet"
    --cov-config=tox.ini
    --no-cov-on-fail
    --color=yes
markers =
    internet: requires connection to the internet
    slow: mark denoting a test that is too slow

How about you? Post your preferred config here and we can try to find an intersection/union :)

[Stannislav]

But I guess in the CI we

• should run all marked tests
• can configure a more complete/verbose output

So, for exampe, in run-tests.yaml we could write

run: tox -vv -e ${{ matrix.tox-env }} -- --color=yes -m "" --cov --durations=20 --verbosity=1