X Tutup
Skip to content

gh-92886: fixed tests that were breaking while running with basic optimizations enabled (-O, assertions off)#93174

Closed
jackh-ncl wants to merge 14 commits intopython:mainfrom
jackh-ncl:92886
Closed

gh-92886: fixed tests that were breaking while running with basic optimizations enabled (-O, assertions off)#93174
jackh-ncl wants to merge 14 commits intopython:mainfrom
jackh-ncl:92886

Conversation

@jackh-ncl
Copy link
Copy Markdown
Contributor

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

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

@ghost
Copy link
Copy Markdown

ghost commented May 24, 2022 •
edited by ghost
Loading

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-bot
Copy link
Copy Markdown

bedevere-bot commented May 24, 2022

Most changes to Python require a NEWS entry.

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

@jackh-ncl jackh-ncl mentioned this pull request May 24, 2022
Closed
10 tasks
jackh-ncl
jackh-ncl commented May 24, 2022
View reviewed changes
Lib/test/_test_multiprocessing.py
@@ -5690,45 +5690,48 @@ def test_joinable_queue(self):

Copy link
Copy Markdown
Contributor Author

@jackh-ncl jackh-ncl May 24, 2022

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?

jackh-ncl
jackh-ncl commented May 24, 2022
View reviewed changes
Lib/test/test_dis.py
code_info_consts = "0: None"
else:
code_info_consts = "0: 'Formatted details of methods, functions, or code.'"

Copy link
Copy Markdown
Contributor Author

@jackh-ncl jackh-ncl May 24, 2022

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.

jackh-ncl
jackh-ncl commented May 24, 2022
View reviewed changes
Lib/test/test_imaplib.py

@threading_helper.reap_threads
@cpython_only
@unittest.skipUnless(__debug__, "Won't work if __debug__ is False")
Copy link
Copy Markdown
Contributor Author

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

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.

jackh-ncl
jackh-ncl commented May 24, 2022
View reviewed changes
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'
Copy link
Copy Markdown
Contributor Author

@jackh-ncl jackh-ncl May 24, 2022

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 🤷‍♂️

jackh-ncl
jackh-ncl commented May 24, 2022
View reviewed changes
Lib/test/test_sys_settrace.py
(13, 'line'),
(13, 'return')])
(10, 'line')] +
([(13, 'line'), (13, 'return')] if __debug__ else [(10, 'return')]))
Copy link
Copy Markdown
Contributor Author

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

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.

@jackh-ncl jackh-ncl marked this pull request as ready for review May 24, 2022 14:59
@AlexWaygood AlexWaygood added the tests Tests in the Lib/test dir label May 24, 2022
@erlend-aasland
Copy link
Copy Markdown
Contributor

erlend-aasland commented May 25, 2022

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
Copy link
Copy Markdown
Contributor Author

jackh-ncl commented May 25, 2022

@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.

@erlend-aasland
Copy link
Copy Markdown
Contributor

erlend-aasland commented May 26, 2022

@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?

@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

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)

4 participants

@jackh-ncl @bedevere-bot @erlend-aasland @AlexWaygood
X Tutup