Tests do not reflect real-world CLI file-discovery behavior

[aaossa]

Describe the bug

This issue is more a heads-up than a bug report.

Follow up of conversation at #3385. Seems like the test suite does not emulate real-world CLI usage. The paths get mixed up somewhere. This may be hiding some bugs: while writing #3385 I wrote a test to detect a confirmed bug (#3384 ) but the test was not failing, so I had to write a test with some patches to emulate the target behavior.

More details

When trying to reproduce #3384 in the test suite I wasn't able to because of an exception. I explained a bit in this comment:

Seems like this can't be tested easily because the exception can't be reproduced in the test suite:

black.get_sources(
    ctx=ctx,
    src=("./dir",),
    quiet=False,
    verbose=True,
    include=DEFAULT_INCLUDE,
    exclude=DEFAULT_EXCLUDE,
    report=report,
    extend_exclude=None,
    force_exclude=None,
    stdin_filename=None,
)

This function call is similar to the call from the CLI, but the test suite fails because src is an invalid value (is_dir() returns false), because every test uses assert_collected_sources and passes an absolute path to the source/target files. But, in reality, the CLI receives relative paths. This means that the test suite is not reproducing the CLI behavior 100% correctly. This seems like another issue, related to #3040 (comment) but not the same.

This reported bug does not happen in the test, because (as you can see in the patch) the solution uses the absolute path to the target file.

Originally posted by @aaossa in #3384 (comment)

This resulted in an ugly (but working and correct) test at #3385 .

As you can see, I'm not the first person that noticed that the tests do not reflect the actual CLI usage. This comment from #3040 reports another issue related to file-collection:

I've been trying to write a test for this and I'm concerned that the existing exclude tests do not reflect real-world usage.

The existing tests make use of the assert_collected_sources() function:

black/tests/test_black.py

Line 1836 in 6c1bd08

def assert_collected_sources(

which in-term makes a call to get_sources():

black/src/black/__init__.py

Line 595 in 6c1bd08

def get_sources(

However, the result of get_sources() is dependent on ctx.obj["root"]. That root folder is set dynamically when you run black using the find_project_root() function:

black/src/black/__init__.py

Lines 458 to 459 in 6c1bd08

	root, method = find_project_root(src) if code is None else (None, None)
	ctx.obj["root"] = root

However, in all of the include/exclude tests the root folder is manually set. None of the tests will reproduce the behaviour that occurs if find_project_root() finds a different folder, which is the behaviour when you actually use black.

Originally posted by @janjachnik-dyson in #3040 (comment)

Where to look

Seems like this requires looking deeper into assert_collected_sources and looking deeper into the context object. Probably a subset of the changes at #3116 🤔

Expected behavior

The tests should be able to emulate the CLI behavior as accurately as possible.

Environment

• Black's version: main
• OS and Python version: NA.