move test fixtures into conftest.py from fixtures.py

[williballenthin]

as described in this thread (astral-sh/ruff#4046), ruff (and other linters) complain about imports that are made in order to provide fixtures to a pytest test. for example, we do things like:

from fixtures import _039a6_dotnetfile_extractor

def test_dotnet(_039a6_dotnetfile_extractor):
    ...

ruff complains with:

tests/test_main.py:xxx:yy: F811 Redefinition of unused `_039a6_dotnetfile_extractor` from line xxx

as suggested in the linked issue, we should move our fixtures into a file named conftest.py, which pytest will use to autodiscovery fixture names.

after we make this change, we should also update any relevant linter configuration that ignores "F401" or "F811".

[AmanSal1]

@williballenthin So basically, what I understood from your explanation is that we need to copy the function with the decorator "@pytest.fixture" from the "fixtures.py" file and move it to the "conftest.py" file. After that, we have to modify the linter's configuration file to ignore the "F401" or "F811" errors.

If I am right can I try to fix this ?

[williballenthin]

not quite: please rename the file "fixtures.py" to "conftest.py". then it should be possible to remove many of the imports that currently reference "fixtures".

i'm not exactly sure precisely the changes to be name, but the above is approximately correct. i'm happy to lend a hand if you hit and problems. please feel welcome to take this issue!

however, you may want to wait until Monday when I can finish and merge the PR that improves the code linters. these will make it more obvious when this issue is fixed.

[AmanSal1]

Oh yes just let me know when you make a successful PR so I can start working!!

Will discuss with you more about the changes after monday

[williballenthin]

ok, so #1591 has been merged, which updates our linter config to be easier to use and more thorough. notably, it includes checks that help demonstrate the issue here. that will make it easier to fix.

the goal is to remove these lines:

capa/.github/ruff.toml

Lines 46 to 61 in 506d677

	# until we address #1592 and move test fixtures into conftest.py
	# then we need to ignore imports done to enable pytest fixtures.
	#
	# F401: `foo` imported but unused
	# F811 Redefinition of unused `foo`
	"tests/test_main.py" = ["F401", "F811"]
	"tests/test_proto.py" = ["F401", "F811"]
	"tests/test_freeze.py" = ["F401", "F811"]
	"tests/test_function_id.py" = ["F401", "F811"]
	"tests/test_viv_features.py" = ["F401", "F811"]
	"tests/test_binja_features.py" = ["F401", "F811"]
	"tests/test_pefile_features.py" = ["F401", "F811"]
	"tests/test_dnfile_features.py" = ["F401", "F811"]
	"tests/test_dotnet_features.py" = ["F401", "F811"]
	"tests/test_result_document.py" = ["F401", "F811"]
	"tests/test_dotnetfile_features.py" = ["F401", "F811"]

so i'd suggest removing those lines and then invoking the linter and trying to get it to pass. you can invoke the ruff linter like this:

$ pre-commit run --all-files ruff

more background on the issue:

we use pytest to invoke our tests. some of our tests use "fixtures", that is, data loaded from the system to help enable a test to work. for example, a PE file that we analyze and assert matches a rule.

a pytest test case looks like this:

def test_foo():
    assert 1 == 1

when there's a fixture, its passed as an argument to the test function:

def test_foo_with_fixture(foo):
    assert foo == 1

pytest inspects the name of the argument and tries to find a symbol in the current scope with that same name and uses this as the fixture.

today we keep our fixtures in the file fixtures.py and import them into the test files like from fixtures import foo which brings the symbol into scope. however, because the thing "foo" isn't directly referenced by the code (only indirectly via the function argument name), linters raise warnings. this is what we want to fix.

from the above linked github issues, pytest recommends putting test fixtures into a file named conftest.py and it will auto-load fixtures from here. then we don't have to have the explicit imports from fixtures.py and the linters won't complain. this is your job @AmanSal1 :-)

in the past, we liked the explicit imports because it makes more clear where the test data is coming from. but today, i think pytest is well known enough that we can expect devs to understand the use of conftest.py.

make sense? any questions?
im happy to help if you need a hand - just let me know.

[williballenthin]

based on the following article i think we can keep the fixtures.py file and add a conftest.py file with the contents from fixtures import *. i like the balance of this solution in that the fixtures are clearly in the fixtures.py file and magic is minimized.

https://www.revsys.com/tidbits/pytest-fixtures-are-magic/