GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-92886: fixed tests that were breaking while running with basic optimizations enabled (-O, assertions off) by jackh-ncl · Pull Request #93174 · python/cpython

Conversation

@jackh-ncl

@jackh-ncl jackh-ncl commented May 24, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Resolves #92886

$ uname -a
Darwin ... 21.4.0 Darwin Kernel Version 21.4.0: Fri Mar 18 00:47:26 PDT 2022; root:xnu-8020.101.4~15/RELEASE_ARM64_T8101 arm64

$ ./python.exe -Om test -j8
...
== Tests result: SUCCESS ==

407 tests OK.

29 tests skipped:
    test_curses test_dbm_gnu test_devpoll test_epoll test_gdb
    test_idle test_ioctl test_launcher test_msilib
    test_multiprocessing_fork test_ossaudiodev test_smtpnet
    test_socketserver test_spwd test_ssl test_startfile test_tcl
    test_tix test_tk test_ttk_guionly test_ttk_textonly test_turtle
    test_urllib2net test_urllibnet test_winconsoleio test_winreg
    test_winsound test_xmlrpc_net test_zipfile64

Total duration: 3 min 49 sec
Tests result: SUCCESS

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Jack Hindmarch added 11 commits May 24, 2022 11:44
Jack Hindmarch
Fixed clinic and dis test
0f8ae9b
Jack Hindmarch
Fixed coroutines test
6afde94
Jack Hindmarch
Skipped test_imaplib.py ThreadedNetworkedTests.test_dump_ur if not in…
0d71b7a 
… debug mode
Jack Hindmarch
fix test_multiprocessing_forkserver.py test
f515d9f
Jack Hindmarch
Using self.assertEqual in test_coroutines.py
ff6b191
Jack Hindmarch
Brining other assertions in line in _test_multiprocessing.py
3926165
Jack Hindmarch
Fixed test_py_compile.py broken test
47a0eb1
Jack Hindmarch
Fixed test_sys_settrace.py tests
1dcb59e
Jack Hindmarch
Fixed handlers.py tests
de758bb
Jack Hindmarch
Fixed other test_py_compile.py test
afb2620
Jack Hindmarch
Fixed test_zipimport.py test
6edf183
@ghost

ghost commented May 24, 2022 •
edited by ghost
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.
CLA signed
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot

bedevere-bot commented May 24, 2022

Copy link
Copy Markdown

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot added the awaiting review label May 24, 2022
@blurb-it
📜🤖 Added by blurb_it.
932449f
@jackh-ncl jackh-ncl mentioned this pull request May 24, 2022
some tests fail with -O (assertions off) #92886
Closed
10 tasks done
jackh-ncl
jackh-ncl commented May 24, 2022
View reviewed changes
Comment thread Lib/test/_test_multiprocessing.py
@@ -5690,45 +5690,48 @@ def test_joinable_queue(self):

@jackh-ncl jackh-ncl May 24, 2022

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Could be worth fixing the rest of the assertions in this file?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
jackh-ncl
jackh-ncl commented May 24, 2022
View reviewed changes
Comment thread Lib/test/test_dis.py
code_info_consts = "0: None"
else:
code_info_consts = "0: 'Formatted details of methods, functions, or code.'"

@jackh-ncl jackh-ncl May 24, 2022

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Failing tests were:

image

It seemed as though this condition was not necessary anymore, though I'm unsure if it is the expected behaviour.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
jackh-ncl
jackh-ncl commented May 24, 2022
View reviewed changes
Comment thread Lib/test/test_imaplib.py

@threading_helper.reap_threads
@cpython_only
@unittest.skipUnless(__debug__, "Won't work if __debug__ is False")

@jackh-ncl jackh-ncl May 24, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

_mesg() @ Lib/imaplib.py:1247 is defined within a if __debug__ block, so I assume this test can be skipped when running with optimizations.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
jackh-ncl
jackh-ncl commented May 24, 2022
View reviewed changes
Comment thread Lib/test/test_py_compile.py
# assert_python_* helpers don't return proc object. We'll just use
# subprocess.run() instead of spawn_python() and its friends to test
# stdin support of the CLI.
opts = '-m' if __debug__ else '-Om'

@jackh-ncl jackh-ncl May 24, 2022

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I'm not sure how this actually fixed things, but it seems to have done the trick 🤷‍♂️

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
jackh-ncl
jackh-ncl commented May 24, 2022
View reviewed changes
Comment thread Lib/test/test_sys_settrace.py
(13, 'line'),
(13, 'return')])
(10, 'line')] +
([(13, 'line'), (13, 'return')] if __debug__ else [(10, 'return')]))

@jackh-ncl jackh-ncl May 24, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The assertions on line 819 affect the line numbers we are testing here when running with optimizations.
An alternative could be for these test functions to do something other than an assert.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Jack Hindmarch added 2 commits May 24, 2022 15:54
Jack Hindmarch
Fix condition in handlers.py
84563b0
Jack Hindmarch
Merge branch '92886' of github.com:jackh-ncl/cpython into 92886
3346ca3
@jackh-ncl
jackh-ncl marked this pull request as ready for review May 24, 2022 14:59
@jackh-ncl
jackh-ncl requested review from a team, brettcannon, encukou, ericsnowcurrently, ncoghlan and warsaw as code owners May 24, 2022 14:59
@AlexWaygood AlexWaygood added the tests Tests in the Lib/test dir label May 24, 2022
@erlend-aasland

erlend-aasland commented May 25, 2022

Copy link
Copy Markdown
Contributor

Please limit this PR to tests only (files in the Lib/test directory). It may make sense to break this up in multiple PRs.
jackh-ncl reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@jackh-ncl

jackh-ncl commented May 25, 2022

Copy link
Copy Markdown
Contributor Author

@erlend-aasland I've split it out into several PRs, which hopefully doesn't seem too overkill. I can see there being some level of back and forth for a number of them.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@erlend-aasland

erlend-aasland commented May 26, 2022

Copy link
Copy Markdown
Contributor

@erlend-aasland I've split it out into several PRs, which hopefully doesn't seem too overkill. I can see there being some level of back and forth for a number of them.

Thanks. One PR per file was perhaps a little bit too much, but IMO it is better than one PR for everything.

IIUC, this PR is now superfluous and can be closed?
jackh-ncl reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@erlend-aasland erlend-aasland added the pending The issue will be closed if no feedback is provided label May 26, 2022
@jackh-ncl jackh-ncl closed this May 26, 2022
@erlend-aasland erlend-aasland removed the pending The issue will be closed if no feedback is provided label May 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@brettcannon brettcannon Awaiting requested review from brettcannon brettcannon is a code owner
@encukou encukou Awaiting requested review from encukou
@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently is a code owner
@ncoghlan ncoghlan Awaiting requested review from ncoghlan ncoghlan is a code owner
@warsaw warsaw Awaiting requested review from warsaw warsaw is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

awaiting review tests Tests in the Lib/test dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

some tests fail with -O (assertions off)

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@jackh-ncl @bedevere-bot @erlend-aasland @AlexWaygood
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.