Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src/sage/env.py: canonicalize paths in a test #38826

Merged
merged 1 commit into from
Oct 26, 2024

Conversation

orlitzky
Copy link
Contributor

A test in sage.env is running sage in a subprocess to compare the values of SAGE_ROOT and SAGE_LOCAL. It does the comparison as strings, however, and can fail:

File "src/sage/env.py", line 14, in sage.env
Failed example:
    out == repr((SAGE_ROOT, SAGE_LOCAL))   # long time
Expected:
    True
Got:
    False

This despite the fact that both values are equivalent:

sage: out
"('/home/mjo/src/sage.git/src/sage/../..', '/usr')"
sage: repr((SAGE_ROOT, SAGE_LOCAL))
"('/home/mjo/src/sage.git', '/usr')"

We update the test to canonicalize the paths within the subprocess, and output only "True" or "False" instead.

@orlitzky
Copy link
Contributor Author

Oh, so that's why it was using f-string substitution for a command with no variables...

Linting files... ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 100% 0:00:09
╭─ Error: namespace_pkg_all_import: import from .all of a namespace package ───╮
│      9     sage: env = {k:v for (k,v) in os.environ.items() if not k.starts  │
│     10     sage: from subprocess import check_output                         │
│  ❱  11     sage: cmd  =  "from sage.all import SAGE_ROOT, SAGE_LOCAL;"       │

A test in sage.env is running sage in a subprocess to compare the
values of SAGE_ROOT and SAGE_LOCAL. It does the comparison as strings,
however, and can fail:

  File "src/sage/env.py", line 14, in sage.env
  Failed example:
      out == repr((SAGE_ROOT, SAGE_LOCAL))   # long time
  Expected:
      True
  Got:
      False

This despite the fact that both values are equivalent:

  sage: out
  "('/home/mjo/src/sage.git/src/sage/../..', '/usr')"
  sage: repr((SAGE_ROOT, SAGE_LOCAL))
  "('/home/mjo/src/sage.git', '/usr')"

We update the test to canonicalize the paths within the subprocess,
and output only "True" or "False" instead.
Copy link

Documentation preview for this PR (built with commit 53a6d31; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@orlitzky
Copy link
Contributor Author

CI is an unrelated timeout:

----------------------------------------------------------------------
sage -t --long --random-seed=286735480429121101562228604801325644303 src/sage/coding/linear_code.py  # Timed out
----------------------------------------------------------------------

I'm going to start shamelessly plugging,

On every ticket where the CI "fails" due to slow tests.

vbraun pushed a commit to vbraun/sage that referenced this pull request Oct 23, 2024
A test in sage.env is running sage in a subprocess to compare the values
of `SAGE_ROOT` and `SAGE_LOCAL`. It does the comparison as strings,
however, and can fail:

```
File "src/sage/env.py", line 14, in sage.env
Failed example:
    out == repr((SAGE_ROOT, SAGE_LOCAL))   # long time
Expected:
    True
Got:
    False
```

This despite the fact that both values are equivalent:

```
sage: out
"('/home/mjo/src/sage.git/src/sage/../..', '/usr')"
sage: repr((SAGE_ROOT, SAGE_LOCAL))
"('/home/mjo/src/sage.git', '/usr')"
```

We update the test to canonicalize the paths within the subprocess, and
output only "True" or "False" instead.

URL: sagemath#38826
Reported by: Michael Orlitzky
Reviewer(s): Tobias Diez
@vbraun vbraun merged commit 2304cfd into sagemath:develop Oct 26, 2024
19 of 20 checks passed
@antonio-rojas
Copy link
Contributor

This breaks the test on distro builds where SAGE_ROOT is not defined

> /usr/bin/python -c "from sage.all import SAGE_ROOT, SAGE_LOCAL;from os.path import samefile;s1 = samefile(SAGE_ROOT, 'None');"

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "<frozen genericpath>", line 112, in samefile
TypeError: stat: path should be string, bytes, os.PathLike or integer, not NoneType

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants