Tests fail due to relative path in PATH env variable

Django Internals
1

AdminScriptTestCase.path_without_formatters fails with the error “ValueError: Can’t mix absolute and relative paths”. This is because there are relative paths in my PATH variable and os.path.commonpath cannot take both absolute and relative paths.

I’m on macOS, using zsh. An example of a relative path, ~/.duckdb/cli/latest.

A workaround is filtering out relative paths before passing to commonpath.

Test report:

FAILED (errors=205, skipped=1651, expected failures=5)

The offending method: django/tests/admin_scripts/tests.py at main · django/django · GitHub

Traceback:

Traceback (most recent call last):
  File "/Users/jonathan/Projects/django/tests/admin_scripts/tests.py", line 2534, in test_option_then_setting
    self._test(args)
    ~~~~~~~~~~^^^^^^
  File "/Users/jonathan/Projects/django/tests/admin_scripts/tests.py", line 2554, in _test
    out, err = self.run_manage(args)
               ~~~~~~~~~~~~~~~^^^^^^
  File "/Users/jonathan/Projects/django/tests/admin_scripts/tests.py", line 194, in run_manage
    return self.run_test(
           ~~~~~~~~~~~~~^
        ["./manage.py", *args],
        ^^^^^^^^^^^^^^^^^^^^^^^
        settings_file,
        ^^^^^^^^^^^^^^
        discover_formatters=discover_formatters,
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    )
    ^
  File "/Users/jonathan/Projects/django/tests/admin_scripts/tests.py", line 151, in run_test
    test_environ["PATH"] = self.path_without_formatters
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/jonathan/Projects/django/django/utils/functional.py", line 47, in __get__
    res = instance.__dict__[self.name] = self.func(instance)
                                         ~~~~~~~~~^^^^^^^^^^
  File "/Users/jonathan/Projects/django/tests/admin_scripts/tests.py", line 122, in path_without_formatters
    if os.path.commonpath([path_component, formatter_path]) == os.sep
       ~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<frozen posixpath>", line 566, in commonpath
ValueError: Can't mix absolute and relative paths
2

A possible solution is resolving path_components in $PATH with os.path.realpath(). This would ensure all paths are absolute.

Should I file a ticket?

3

Hey @villager, thank you for your post! This was caught over night and fixed by Mariusz in Refs #36680 -- Fixed admin_scripts tests crash when black is not installed. by felixxm · Pull Request #20023 · django/django · GitHub.

If you pull latest main, this should be solved. :partying_face:

4

Hi @nessita , thanks for your quick response! I pulled the changes and am still encountering the error. The issue occurs in the same function, but is not related to Mariusz’ fix.

5

Hello @villager,

From the stacktrace you shared, it really seems the same issue. Do you have more information for us to reproduce?

6

Hi @nessita, yes definitely. If you add a relative path to $PATH and then run the tests I believe it will produce the error. For example, this portion of the test suite fails after 3 tests for me:

export PATH='~/test':$PATH
python tests/runtests.py --parallel=1 --failfast ./tests/admin_scripts/
1 Like
7

For more context, here is the change I made so I can run the entire suite without any unexpected errors. Though, maybe a proper fix should coerce all paths in $PATH to be absolute.

    @cached_property
    def path_without_formatters(self):
        return os.pathsep.join(
            [
                path_component
                for path_component in os.environ.get("PATH", "").split(os.pathsep)
                for formatter_path in find_formatters().values()
                if formatter_path
+               and os.path.isabs(path_component)
                and os.path.commonpath([path_component, formatter_path]) == os.sep
            ]
        )
8

Hi @villager, thanks for jumping on this. Could you open a PR with the most appropriate fix in your opinion, here? Thanks :sunny:

1 Like
9

PR opened. Thanks!

10

I suspect the actual problem here is not relative paths, but non-existent ones. Unless you are trying to use a directory literally named ~ (the tilde character).

In most (but not all) shells, ~ expands to your home directory. So ~/test is a shortcut for for the absolute path /Users/jonathan/test. (See, e.g., Bash’s tilde expansion.)

However, single quotes disable shell expansions, so writing:

export PATH='~/test':$PATH`

means you want your PATH to include “~/test” relative to the current working directory. (Again, with a directory actually named ~.)

You likely meant to write:

export PATH=~/test:$PATH

(without the single quotes).

If there’s a bug in Django’s tests here, the correct fix is probably to check os.path.exists().

(Python also has os.path.expanduser() that mimics some shell tilde expansion, but it wouldn’t really be appropriate to use it in this case. The PATH environment variable is expected to be fully resolved by the time this code goes through it.)

11

Hi @medmunds, thanks for pointing out that I can remove the single quotes. It does seem to work fine if I leave them in, I’m using zsh for context.

You’re right that checking if os.path.exists() would also solve the issue in my particular case.
Because os.path.exists("~/test") evaluates to False even when /Users/jonathan/test does exist

However it would not work for developers who have a relative path like ../test in their PATH. For that reason, and as far as I can tell having a path prepended with ~ is valid, I still believe checking with os.path.isabs() is the correct solution.

Additionally, absolute but non-existent paths are not a problem here because they would not share a commonality with the formatter_path.

I’m also happy to accept that this is just something I should fix in my PATH because the problem is so rare it is not worth the extra code.

12

Ah, you’re absolutely correct about that. Something like ../test or ./scripts is a valid relative PATH component. And you’re correct that os.path.isabs() is the right fix for that case. I’ll let the Django Fellows decide whether it needs fixing.

Having a literal ~ in your PATH environment variable, although technically valid, is not going to produce the expected results (unless you truly have a directory named ~ and are trying to refer to it). zsh has the same behavior as bash here: ~ homedir expansion only works with unquoted tildes.

1 Like