◐ Shell
reader mode source ↗
Skip to content

bpo-32374: Fix test_bad_traverse.#7145

Merged
encukou merged 2 commits into
masterfrom
unknown repository
May 28, 2018
Merged

bpo-32374: Fix test_bad_traverse.#7145
encukou merged 2 commits into
masterfrom
unknown repository

Conversation

@ghost

@ghost ghost commented May 28, 2018 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown

Previous test assuring that incorrect handling of m_traverse "shouts loudly" on crash
passed also when Python exception was raised.

Fix using try-except block is proposed.

https://bugs.python.org/issue32374

encukou
encukou approved these changes May 28, 2018
View reviewed changes

@encukou encukou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hide comment

Right, a test for a C-level crash should ignore Python-level exceptions.

vstinner
vstinner approved these changes May 28, 2018
View reviewed changes

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hide comment

LGTM, I just have a question/suggestion on the comment.

@vstinner

vstinner commented May 28, 2018

Copy link
Copy Markdown
Member

I proposed a different approach, explicitly reject exit code 1: PR #7147.

vstinner
vstinner reviewed May 28, 2018
View reviewed changes

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hide comment

LGTM.

@encukou encukou merged commit 08c5aca into python:master May 28, 2018
@miss-islington

miss-islington commented May 28, 2018

Copy link
Copy Markdown
Contributor

Thanks @Traceur759 for the PR, and @encukou for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

@bedevere-bot

bedevere-bot commented May 28, 2018

Copy link
Copy Markdown

GH-7150 is a backport of this pull request to the 3.7 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 28, 2018
@miss-islington
bpo-32374: Ignore Python-level exceptions in test_bad_traverse (pytho… …
f0f52c8 
…nGH-7145)

(cherry picked from commit 08c5aca)

Co-authored-by: Marcel Plch <gmarcel.plch@gmail.com>
encukou pushed a commit that referenced this pull request May 28, 2018
@miss-islington @encukou
bpo-32374: Ignore Python-level exceptions in test_bad_traverse (GH-7145… …
983c1ba 
…) (GH-7150)

(cherry picked from commit 08c5aca)

Co-authored-by: Marcel Plch <gmarcel.plch@gmail.com>
@vstinner

vstinner commented May 28, 2018

Copy link
Copy Markdown
Member

The change should also be backported to Python 3.6, no? @encukou

@ghost

ghost commented May 28, 2018

Copy link
Copy Markdown
Author

@vstinner Python 3.6 does not have features which the test is written against.

@vstinner

vstinner commented May 28, 2018

Copy link
Copy Markdown
Member

@vstinner Python 3.6 does not have features which the test is written against.

I'm talking about this code in the 3.6 branch:
https://github.com/python/cpython/blob/3.6/Lib/test/test_importlib/extension/test_loader.py#L270-L285

It's the commit 1da0479 of bpo-32374.

Which feature is missing from 3.6?

@ghost

ghost commented May 28, 2018

Copy link
Copy Markdown
Author

@vstinner Sorry, my bad, I had outdated local branch and didn't see the code there. Backporting is a valid question.

@miss-islington

miss-islington commented May 28, 2018

Copy link
Copy Markdown
Contributor

Thanks @Traceur759 for the PR, and @encukou for merging it 🌮🎉.. I'm working now to backport this PR to: 3.6.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 28, 2018
@miss-islington
bpo-32374: Ignore Python-level exceptions in test_bad_traverse (pytho… …
8378791 
…nGH-7145)

(cherry picked from commit 08c5aca)

Co-authored-by: Marcel Plch <gmarcel.plch@gmail.com>
@bedevere-bot

bedevere-bot commented May 28, 2018

Copy link
Copy Markdown

GH-7155 is a backport of this pull request to the 3.6 branch.

@vstinner

vstinner commented May 28, 2018

Copy link
Copy Markdown
Member

@vstinner Sorry, my bad, I had outdated local branch and didn't see the code there. Backporting is a valid question.

I asked @miss-islington to create a PR, I tested it: it works as expected, so I approved the PR #7155. I will be merged as soon as tests pass.

miss-islington added a commit that referenced this pull request May 28, 2018
@miss-islington
bpo-32374: Ignore Python-level exceptions in test_bad_traverse (GH-7145) …
6ec325d 
(cherry picked from commit 08c5aca)

Co-authored-by: Marcel Plch <gmarcel.plch@gmail.com>
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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@vstinner @miss-islington @bedevere-bot @encukou @bitdancer @the-knights-who-say-ni