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

Fix skipped tests & remove deprecation warnings #6018

Merged
merged 2 commits into from
Mar 24, 2021

Conversation

befeleme
Copy link
Contributor

Hey,
I propose some changes to the tests:

  1. Unskip tests using obsolete pytest yield feature. That is changed to recommended pytest.mark.parametrize.
  2. Remove DeprecationWarning caused by \d in regular expression - use recommended raw string instead.
  3. Remove PytestCollectionWarning failing on recognizing TestingStopApp - problematic part is ^Test* in the class name - renaming makes it work again.

Copy link
Member

@kevin-bates kevin-bates left a comment

Choose a reason for hiding this comment

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

Thank you @befeleme - these changes look great. I had the comment about the stop app class name so that the test subject (StopApp) is preserved.

@@ -139,7 +137,7 @@ def test_notebook_password():
assert nb.password != ''
passwd_check(nb.password, password)

class TestingStopApp(notebookapp.NbserverStopApp):
class StopTestApp(notebookapp.NbserverStopApp):
Copy link
Member

Choose a reason for hiding this comment

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

Since the purpose of this is to test the "StopApp", I would suggest a rename to StopAppTest.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for pointing this out, fixing it.

Copy link
Member

@kevin-bates kevin-bates left a comment

Choose a reason for hiding this comment

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

Looks good. Thank you.

@kevin-bates kevin-bates merged commit c719e27 into jupyter:master Mar 24, 2021
@befeleme befeleme deleted the fix-tests branch March 24, 2021 07:04
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants