psf / psf/black

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

Open
#3,552 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

T: bug
Dominant language
Python
Stars
41.8k
Forks
2.9k
Avg merge
5d 6h
Merged PRs (30d)
9

Description

Describe the bug

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

Follow up of conversation at https://github.com/psf/black/pull/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 https://github.com/psf/black/issues/3040#issuecomment-1157792654 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 https://github.com/psf/black/issues/3384#issuecomment-1314295118

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 https://github.com/psf/black/issues/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: https://github.com/psf/black/blob/6c1bd08f16b636de38b92aeb2e0a1e8ebef0a0b1/tests/test_black.py#L1836 which in-term makes a call to get_sources(): https://github.com/psf/black/blob/6c1bd08f16b636de38b92aeb2e0a1e8ebef0a0b1/src/black/__init__.py#L595

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: https://github.com/psf/black/blob/6c1bd08f16b636de38b92aeb2e0a1e8ebef0a0b1/src/black/__init__.py#L458-L459

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 https://github.com/psf/black/issues/3040#issuecomment-1157792654

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 https://github.com/psf/black/pull/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.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with assert_collected_sources in tests/test_black.py and trace its calls to get_sources in src/black/init.py. Compare the tests' manually set context and absolute paths with the CLI's find_project_root behavior and relative paths. Done means file-discovery tests reproduce CLI behavior accurately and cover the reported path-handling cases.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
cli, testing-qa
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.