Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

gh-94751: Install, import and run the test C++ extension (MVP) #94754

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jul 12, 2022
Merged

gh-94751: Install, import and run the test C++ extension (MVP) #94754

merged 2 commits into from
Jul 12, 2022

Conversation

encukou
Copy link
Member

@encukou encukou commented Jul 11, 2022 •
edited by bedevere-bot
Loading

This is a quick-and-dirty way to run the C++ tests. It can definitely be improved in the future, but it should fail when things go wrong.

  • Run test functions on import (yes, this can definitely be improved)
  • Fudge setuptools metadata (name & version) to make the extension installable
  • Install and import the extension in test_cppext

@encukou
bpo-94751: Install, import and run the test C++ extension (MVP)
f3d66db 
- Run test functions on import (yes, this can definitely be improved)
- Fudge setuptools metadata (name & version) to make the extension
  installable
- Install and import the extension in test_cppext
@encukou encukou added tests Tests in the Lib/test dir skip news needs backport to 3.11 only security fixes labels Jul 11, 2022
@encukou encukou changed the title bpo-94751: Install, import and run the test C++ extension (MVP) gh-94751: Install, import and run the test C++ extension (MVP) Jul 11, 2022
@encukou encukou mentioned this pull request Jul 11, 2022
Closed
@da-woods
Copy link
Contributor

It looks good to me as based on a quick read through. I think it's definitely worth running the tests rather than assuming that it's fine if it compiles.

It looks like '-X', 'showrefcount', is just going to require inspecting by eye? That's probably reasonable to defer for a future improvement, but it obviously reduces the chance of it catching any issues.

@encukou
Copy link
Member Author

encukou commented Jul 11, 2022

Thank you!

It looks like '-X', 'showrefcount', is just going to require inspecting by eye? That's probably reasonable to defer for a future improvement

Yes. I filed #94755 for the future improvement.

@encukou encukou added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 11, 2022
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @encukou for commit a615602 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 11, 2022
@encukou encukou merged commit ec5db53 into python:main Jul 12, 2022
@miss-islington
Copy link
Contributor

Thanks @encukou for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

@encukou encukou deleted the cppext-run branch July 12, 2022 15:06
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 12, 2022
@encukou @miss-islington
pythongh-94751: Install, import and run the test C++ extension (MVP) (p…
1ab009d 
…ythonGH-94754)

This is a quick-and-dirty way to run the C++ tests.
It can definitely be improved in the future, but it should fail when things go wrong.

- Run test functions on import (yes, this can definitely be improved)
- Fudge setuptools metadata (name & version) to make the extension installable
- Install and import the extension in test_cppext
(cherry picked from commit ec5db53)

Co-authored-by: Petr Viktorin <encukou@gmail.com>
@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Jul 12, 2022
@bedevere-bot
Copy link

GH-94780 is a backport of this pull request to the 3.11 branch.

@da-woods
Copy link
Contributor

I'm seeing segfaults on from test_cppext on what should be an unrelated PR https://github.com/python/cpython/runs/7316060423?check_suite_focus=true

======================================================================
ERROR: test_build_cpp03 (test.test_cppext.TestCPPExt.test_build_cpp03)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/runner/work/cpython/cpython-ro-srcdir/Lib/test/test_cppext.py", line 23, in test_build_cpp03
    self.check_build(True, '_testcpp03ext')
  File "/home/runner/work/cpython/cpython-ro-srcdir/Lib/test/test_cppext.py", line 33, in check_build
    self._check_build(std_cpp03, extension_name)
  File "/home/runner/work/cpython/cpython-ro-srcdir/Lib/test/test_cppext.py", line 95, in _check_build
    run_cmd('Import', cmd)
  File "/home/runner/work/cpython/cpython-ro-srcdir/Lib/test/test_cppext.py", line 58, in run_cmd
    subprocess.run(cmd, check=True)
  File "/home/runner/work/cpython/cpython-ro-srcdir/Lib/subprocess.py", line 558, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['env/bin/python', '-X', 'dev', '-X', 'showrefcount', '-c', 'import _testcpp03ext']' died with <Signals.SIGSEGV: 11>.

encukou added a commit that referenced this pull request Jul 13, 2022
@miss-islington @encukou
gh-94751: Install, import and run the test C++ extension (MVP) (GH-94754
ffbd6ae 
) (#94780)

This is a quick-and-dirty way to run the C++ tests.
It can definitely be improved in the future, but it should fail when things go wrong.

- Run test functions on import (yes, this can definitely be improved)
- Fudge setuptools metadata (name & version) to make the extension installable
- Install and import the extension in test_cppext
(cherry picked from commit ec5db53)

Co-authored-by: Petr Viktorin <encukou@gmail.com>
@encukou
Copy link
Member Author

encukou commented Jul 13, 2022

You've accidentally included the failing reproducer in that unrelated PR.

Context switching is hard! Thanks for doing it to come up with the reproducer :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
skip news tests Tests in the Lib/test dir
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@encukou @da-woods @bedevere-bot @miss-islington
Morty Proxy This is a proxified and sanitized view of the page, visit original site.