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

gh-91338: Ensure test_ucnhash_capi_reset doesn't pass PYTHONHOME to _testembed.exe #32313

Closed
wants to merge 3 commits into from

Conversation

neonene
Copy link
Contributor

@neonene neonene commented Apr 4, 2022

A minor tweak for Windows. _test_embed.exe fails to import *.pyd modules when the initialization is repeated with PYTHONHOME from python.bat.

>python.bat -m test test_embed -v

======================================================================
FAIL: test_ucnhash_capi_reset (test.test_embed.EmbeddingTests.test_ucnhash_capi_reset)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\cpython\Lib\test\test_embed.py", line 349, in test_ucnhash_capi_reset
    out, err = self.run_embedded_interpreter("test_repeated_init_exec", code)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\cpython\Lib\test\test_embed.py", line 117, in run_embedded_interpreter
stdout:
9

stderr:
--- Loop #1 ---
--- Loop #2 ---
  File "<string>", line 1
    print('\N{digit nine}')
                          ^
SyntaxError: (unicode error) \N escapes not supported (can't load unicodedata module)

sys.path at Loop#1 has the build directory, which is replaced with f"{PYTHONHOME}/DLLs" at the next loop. The behavior looks realistically designed.

GitHub CI machines doesn't show the error, using rt.bat where PYTHONHOME is not set.

https://bugs.python.org/issue47182

@bedevere-bot bedevere-bot added the tests Tests in the Lib/test dir label Apr 4, 2022
@tiran tiran added the skip news label Apr 4, 2022
@tiran
Copy link
Member

tiran commented Apr 4, 2022

Looks good to me!

@neonene
Copy link
Contributor Author

neonene commented Apr 23, 2022

@zooba, could you take a look at this?
I'll close if unnecessary.

@zooba
Copy link
Member

zooba commented Apr 25, 2022

We have a few other tests that use the same function. I'd assume we either need to update all of them (because they're all "wrong", even if they're not triggering it yet), or make sure that the values aren't regenerated differently.

Do we know the root cause of the changed value?

@neonene neonene changed the title bpo-47182: Ensure test_ucnhash_capi_reset doesn't pass PYTHONHOME to _testembed.exe gh-91338: Ensure test_ucnhash_capi_reset doesn't pass PYTHONHOME to _testembed.exe Apr 26, 2022
@neonene
Copy link
Contributor Author

neonene commented Apr 26, 2022

Holding the value of platstdlib_dir for getpath.py would fix the root cause.
My implementation was modifying PyConfig and _PyPathConfig structures.
This PR (updated) is less risky for me.

@neonene
Copy link
Contributor Author

neonene commented Apr 27, 2022

Do we know the root cause of the changed value?

I'll close this and open an issue, considering that the current behavior, which I think is practical, may be unintentional.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting merge skip news tests Tests in the Lib/test dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants