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

test_os.test_win32_mkdir_700 fails on Windows #120164

Closed
Eclips4 opened this issue Jun 6, 2024 · 4 comments
Closed

test_os.test_win32_mkdir_700 fails on Windows #120164

Eclips4 opened this issue Jun 6, 2024 · 4 comments
Labels
OS-windows tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error

Comments

@Eclips4
Copy link
Member

Eclips4 commented Jun 6, 2024

Bug report

Bug description:

./python -m test -v test_os -m test_win32_mkdir_700
Running Debug|x64 interpreter...
== CPython 3.14.0a0 (heads/main-dirty:78634cfa3d, Jun 6 2024, 18:39:33) [MSC v.1933 64 bit (AMD64)]
== Windows-10-10.0.19043-SP0 little-endian
== Python build: debug
== cwd: C:\Users\KIRILL-1\CLionProjects\cpython\build\test_python_worker_6160æ
== CPU count: 16
== encodings: locale=cp1251 FS=utf-8
== resources: all test resources are disabled, use -u option to unskip tests

Using random seed: 3576474721
0:00:00 Run 1 test sequentially in a single process
0:00:00 [1/1] test_os
test_win32_mkdir_700 (test.test_os.MakedirTests.test_win32_mkdir_700) ... FAIL

======================================================================
FAIL: test_win32_mkdir_700 (test.test_os.MakedirTests.test_win32_mkdir_700)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Users\KIRILL-1\CLionProjects\cpython\Lib\test\test_os.py", line 1840, in test_win32_mkdir_700
    self.assertEqual(
    ~~~~~~~~~~~~~~~~^
        out.strip(),
        ^^^^^^^^^^^^
        f'{path} "D:P(A;OICI;FA;;;SY)(A;OICI;FA;;;BA)(A;OICI;FA;;;OW)"',
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    )
    ^
AssertionError: 'C:\\[66 chars]_6160?\\@test_6160_tmp?\\dir "D:P(A;OICI;FA;;;[32 chars]OW)"' != 'C:\\[66 chars]_6160æ\\@test_6160_tmpæ\\dir "D:P(A;OICI;FA;;;[32 chars]OW)"'
- C:\Users\KIRILL-1\CLionProjects\cpython\build\test_python_worker_6160?\@test_6160_tmp?\dir "D:P(A;OICI;FA;;;SY)(A;OICI;FA;;;BA)(A;OICI;FA;;;OW)"
?                                                                      ^               ^
+ C:\Users\KIRILL-1\CLionProjects\cpython\build\test_python_worker_6160æ\@test_6160_tmpæ\dir "D:P(A;OICI;FA;;;SY)(A;OICI;FA;;;BA)(A;OICI;FA;;;OW)"
?                                                                      ^               ^


----------------------------------------------------------------------
Ran 1 test in 0.036s

FAILED (failures=1)
test test_os failed
test_os failed (1 failure)

== Tests result: FAILURE ==

1 test failed:
    test_os

Total duration: 483 ms
Total tests: run=1 (filtered) failures=1
Total test files: run=1/1 (filtered) failed=1
Result: FAILURE

CPython versions tested on:

CPython main branch

Operating systems tested on:

Windows

Linked PRs

@Eclips4 Eclips4 added type-bug An unexpected behavior, bug, or error tests Tests in the Lib/test dir OS-windows labels Jun 6, 2024
@Eclips4
Copy link
Member Author

Eclips4 commented Jun 6, 2024

@zooba I think it's potentially related to #118802 or #118773

@eryksun
Copy link
Contributor

eryksun commented Jun 6, 2024

It's an encoding issue with the output of "cacls.exe". When writing to a pipe, "cacls.exe" uses the process OEM code page. The test system is configured to use the legacy ANSI/OEM code pages of the system locale, and the ANSI code page is 1251 (Cyrillic) according to the initial info displayed by the test. The corresponding OEM code page is 866. The character "æ" isn't mapped by this code page, and the WideCharToMultiByte() call in "cacls.exe" allows the encoded string to use best-fit characters and the default character "?".

On Windows, I'd expect os_helper.TESTFN to be limited to the set of legal filename characters in the intersection of the ANSI and OEM code pages. Instead, it checks that it can roundtrip though os.fsencode() and os.fsdecode(). The filesystem encoding is emulated as UTF-8 on Windows, since internally Python uses the Windows wide-character API. Not limiting TESTFN to ANSI/OEM is reasonable for tests within Python processes that can force piping text as UTF-8, but it's not reasonable when filenames pass through 3rd party applications such as "cacls.exe".

Anyway, instead of opening this can of worms about the definition of TESTFN on Windows, I suggest just changing the test to only compare the last 51 characters to ensure that the DACL is "D:P(A;OICI;FA;;;SY)(A;OICI;FA;;;BA)(A;OICI;FA;;;OW)".

@zooba
Copy link
Member

zooba commented Jun 6, 2024

On Windows, I'd expect os_helper.TESTFN to be limited to the set of legal filename characters in the intersection of the ANSI and OEM code pages

Yeah, we deliberately go outside of ANSI/OEM because we want to make sure that all our APIs are going through the Unicode variants. Unfortunately, cacls is just too tempting to use instead of writing all the code to use the proper native APIs.

I suggest just changing the test to only compare the last 51 characters to ensure that the DACL is "D:P(A;OICI;FA;;;SY)(A;OICI;FA;;;BA)(A;OICI;FA;;;OW)".

Splitting at the last space and comparing what comes afterwards is probably going to give nicer output if/when it fails for real, but yeah, ignoring the path is probably best.

vstinner pushed a commit that referenced this issue Jun 7, 2024
Don't compare the path to avoid encoding issues.

Co-authored-by: Eryk Sun <eryksun@gmail.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jun 7, 2024
Don't compare the path to avoid encoding issues.

(cherry picked from commit d5ba4fc)

Co-authored-by: Kirill Podoprigora <kirill.bast9@mail.ru>
Co-authored-by: Eryk Sun <eryksun@gmail.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jun 7, 2024
Don't compare the path to avoid encoding issues.

(cherry picked from commit d5ba4fc)

Co-authored-by: Kirill Podoprigora <kirill.bast9@mail.ru>
Co-authored-by: Eryk Sun <eryksun@gmail.com>
@vstinner
Copy link
Member

vstinner commented Jun 7, 2024

Fixed by d5ba4fc

@vstinner vstinner closed this as completed Jun 7, 2024
vstinner pushed a commit that referenced this issue Jun 7, 2024
…0203)

gh-120164: Fix test_os.test_win32_mkdir_700() (GH-120177)

Don't compare the path to avoid encoding issues.

(cherry picked from commit d5ba4fc)

Co-authored-by: Kirill Podoprigora <kirill.bast9@mail.ru>
Co-authored-by: Eryk Sun <eryksun@gmail.com>
vstinner pushed a commit that referenced this issue Jun 7, 2024
…0202)

gh-120164: Fix test_os.test_win32_mkdir_700() (GH-120177)

Don't compare the path to avoid encoding issues.

(cherry picked from commit d5ba4fc)

Co-authored-by: Kirill Podoprigora <kirill.bast9@mail.ru>
Co-authored-by: Eryk Sun <eryksun@gmail.com>
noahbkim pushed a commit to hudson-trading/cpython that referenced this issue Jul 11, 2024
Don't compare the path to avoid encoding issues.

Co-authored-by: Eryk Sun <eryksun@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OS-windows tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

4 participants