Test size.py

[hugovk]

No description provided.

[ezio-melotti]

What about doing something like this and reuse files for the next test too, cutting down considerably the number of lines and avoiding duplication?

Suggested change

	@pytest.mark.parametrize(
	["filename", "expected"],
	[
	("file.tgz", False),
	("file.tar.bz2", False),
	("file.tar.xz", False),
	("file.pdb.zip", False),
	("file.amd64.msi", False),
	("file.msi", False),
	("file.chm", False),
	("file.dmg", False),
	("file.ext", True),
	],
	)
	files = ["file.tgz", "file.tar.bz2", "file.tar.xz", "file.pdb.zip",
	"file.amd64.msi", "file.msi", "file.chm", "file.dmg", "file.ext"]
	@pytest.mark.parametrize(
	["filename", "expected"],
	list(zip(files, [False]*8 + [True])),
	)

list(zip(files, [False]*8 + [True])) is not as readable as the original list, but should be understandable, especially if a comment/docstring like "Check that all the listed extensions are ignored except for ext." is added.

Another option could be:

Suggested change

	@pytest.mark.parametrize(
	["filename", "expected"],
	[
	("file.tgz", False),
	("file.tar.bz2", False),
	("file.tar.xz", False),
	("file.pdb.zip", False),
	("file.amd64.msi", False),
	("file.msi", False),
	("file.chm", False),
	("file.dmg", False),
	("file.ext", True),
	],
	)
	files = ["file.tgz", "file.tar.bz2", "file.tar.xz", "file.pdb.zip",
	"file.amd64.msi", "file.msi", "file.chm", "file.dmg", "file.ext"]
	ignored = [False, False, False, False, False, False, False, False, True]
	@pytest.mark.parametrize(
	["filename", "expected"],
	list(zip(files, ignored)),
	)

Maybe the list() can also be omitted.

[hugovk]

Tests are "special" and often benefit from duplication and being more explicit:

Since tests don't have tests, it should be easy for humans to manually inspect them for correctness, even at the expense of greater code duplication. This means that the DRY principle often isn’t a good fit for unit tests, even though it is a best practice for production code.

In tests we can use the DAMP principle (“Descriptive and Meaningful Phrases”), which emphasizes readability over uniqueness. Applying this principle can introduce code redundancy (e.g., by repeating similar code), but it makes tests more obviously correct.

https://testing.googleblog.com/2019/12/testing-on-toilet-tests-too-dry-make.html

https://stackoverflow.com/q/129693/724176

Side-by-side filenames and corresponding expected results are much clearer than having to re-map in your head two unaligned lists, plus checking the list(zip(...)).

[ezio-melotti]

I generally agree with this: that's why I wasn't really happy with list(zip(files, [False]*8 + [True])), why I also suggested to add comments to clarify the intention of the test(s), and why I proposed another alternative that was more readable.

The second alternative is trying to find a middle ground between readability and DRYness:

• it makes it clear that both tests are working with the same list of files;
• it almost halves the total number of lines, making the file more compact and readable;
• it makes the tests easier to maintain, without having to keep multiple lists in sync;

[hugovk]

I think some duplication is fine for tests, to be more standalone and explicit. We probably don't need to update them too often.

[ambv]

I'm with Hugo on this. Please don't replace a trivial list of pairs with implicit pairing. If there was one "True" in the middle, it would be very inconvenient to check which one that is.

[ezio-melotti]

Suggested change

	@pytest.mark.parametrize(
	["filename", "expected"],
	[
	("file.tgz", 0),
	("file.tar.bz2", 1),
	("file.tar.xz", 2),
	("file.pdb.zip", 3),
	("file.amd64.msi", 4),
	("file.msi", 5),
	("file.chm", 6),
	("file.dmg", 7),
	("file.ext", 9999),
	],
	)
	@pytest.mark.parametrize(
	["filename", "expected"],
	list(zip(files, [0, 1, 2, 3, 4, 5, 6, 7, 9999])),
	)