8000 bpo-24813: IDLE: Add bitness to About Idle title by csabella · Pull Request #2380 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

bpo-24813: IDLE: Add bitness to About Idle title #2380

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

Merged
merged 2 commits into from
Jun 27, 2017
Merged

bpo-24813: IDLE: Add bitness to About Idle title #2380

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e3da481
bpo-24813: IDLE: Add bitness to About Idle title
csabella Jun 24, 2017
709fd72
bpo-24813: IDLE: Add bitness to About Idle title
csabella Jun 26, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
bpo-24813: IDLE: Add bitness to About Idle title
  • Loading branch information
@csabella
csabella committed Jun 24, 2017
commit e3da4814088c3e06eab5db74b9e6b5f494fa8d00
18 changes: 14 additions & 4 deletions Lib/idlelib/help_about.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,23 @@

"""
import os
from platform import python_version
import sys
from platform import python_version, architecture

from tkinter import Toplevel, Frame, Label, Button, PhotoImage
from tkinter import SUNKEN, TOP, BOTTOM, LEFT, X, BOTH, W, EW, NSEW, E

from idlelib import textview


def _bitness():
Copy link
Member

Choose a reason for hiding this comment

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

def build_bits():

"Return bitness of platform."
bits, linkage = architecture()
if sys.platform == 'darwin':
bits = '64bit' if sys.maxsize > 2**32 else '32bit'
Copy link
Member

Choose a reason for hiding this comment

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

if sys.platform == 'darwin':
    return '64' if sys.maxsize > 2**32 else '32'
else:
    return  architecture()[0][:2]

I guess I prefer multiple crisp returns to multiple assignments used to make a delayed return. I do understand the single exit idea from the Structured Programming revolution of the 1970s. 'Single exit' replaced multiple unrestricted GOTOs. A return is a very disciplined goto.

Since I have 64 bit installs and 32 bit clone debug builds, I tested and confirmed that the return on windows really is build bits and, in spite of the name, not physical architecture bits.

Copy link
Contributor

Choose a reason for hiding this comment

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

concur with @terryjreedy 's method and name of the function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is much better, thank you.

return bits[:2]


class AboutDialog(Toplevel):
"""Modal about dialog for idle

Expand All @@ -28,11 +37,12 @@ def __init__(self, parent, title=None, _htest=False, _utest=False):
self.geometry("+%d+%d" % (
parent.winfo_rootx()+30,
parent.winfo_rooty()+(30 if not _htest else 100)))
self.bg = "#707070"
self.fg = "#ffffff"
self.bg = "#bbbbbb"
self.fg = "#000000"
self.create_widgets()
self.resizable(height=False, width=False)
self.title(title or f'About IDLE {python_version()}')
self.title(title or
f'About IDLE {python_version()} ({_bitness()} bit)')
self.transient(parent)
self.grab_set()
self.protocol("WM_DELETE_WINDOW", self.ok)
Expand Down
44 changes: 41 additions & 3 deletions Lib/idlelib/idle_test/test_help_about.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,15 @@
from test.support import requires, findfile
from tkinter import Tk, TclError
import unittest
from unittest import mock
from idlelib.idle_test.mock_idle import Func
from idlelib.idle_test.mock_tk import Mbox_func
from idlelib.help_about import AboutDialog as About
from idlelib import help_about
from idlelib import textview
import os.path
from platform import python_version
from platform import python_version, architecture


class LiveDialogTest(unittest.TestCase):
"""Simulate user clicking buttons other than [Close].
Expand Down Expand Up @@ -88,18 +91,21 @@ def setUpClass(cls):
requires('gui')
cls.root = Tk()
cls.root.withdraw()
help_about._bitness = mock.Mock(return_value=42)
cls.dialog = About(cls.root, _utest=True)

@classmethod
def tearDownClass(cls):
del cls.dialog
del cls.dialog, help_about._bitness
cls.root.update_idletasks()
cls.root.destroy()
del cls.root

def test_dialog_title(self):
"""Test about dialog title"""
self.assertEqual(self.dialog.title(), f'About IDLE {python_version()}')
self.assertEqual(self.dialog.title(),
f'About IDLE {python_version()}'
f' (42 bit)')


class CloseTest(unittest.TestCase):
Expand Down Expand Up @@ -170,5 +176,37 @@ def test_file_display(self):
self.assertEqual(self.view.called, True)


class BitnessTest(unittest.TestCase):
"Test helper function for bitness of system."

def test_darwin(self):
help_about.sys = mock.MagicMock()
platform = mock.PropertyMock(return_value='darwin')
size = mock.PropertyMock(return_value=2**32)
type(help_about.sys).platform = platform
type(help_about.sys).maxsize = size
help_about.architecture = mock.Mock(return_value=('64bit', 'ELF'))
self.assertEqual(help_about._bitness(), '32')
platform.assert_called_with()
size.assert_called_with()

size = mock.PropertyMock(return_value=2**64)
type(help_about.sys).maxsize = size
self.assertEqual(help_about._bitness(), '64')
platform.assert_called_with()
size.assert_called_with()

def test_not_darwin(self):
help_about.sys = mock.MagicMock()
platform = mock.PropertyMock(return_value='linux')
size = mock.PropertyMock(return_value=2**32)
type(help_about.sys).platform = platform
type(help_about.sys).maxsize = size
help_about.architecture = mock.Mock(return_value=('64bit', 'ELF'))
self.assertEqual(help_about._bitness(), '64')
platform.assert_called_with()
size.assert_not_called()
Copy link
Member

Choose a reason for hiding this comment

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

I have decided that these tests are way too much for something that almost does 8000 not need to be tested. Testing that the implementation is what we can see it is by reading the code is not very useful, though a common mistake. If the implementation is wrong, it is wrong. Reduce this to testing what we know must be true if there is no bug.
# untested ;-)
def test_build_bits(self)
self.assertIn(help_about.build_bits, (32, 64))

Put is some current class. Rename if needed.

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. I thought I needed to test each branch of the if for coverage. :-( I need to read more about writing good tests. Do you have any suggestions?

Copy link
Member
@terryjreedy terryjreedy Jun 27, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

If build bits were a real function, I would want to to try to hit all branches. And if we otherwise get to to 99'% coverage, I might want to do so, but it would be artificial. On Macs, the value of sys.architecture seems undefined, which is why it is not used. (Or I might just note that 99% is 100% of what is sensibly possible on any one system. ;-)

Read something about 'unit testing', such as parts of https://en.wikipedia.org/wiki/Unit_testing and 'test-driven development', such as the first half of https://en.wikipedia.org/wiki/Test-driven_development. There is clearly overlap. Keep in mind that both topics are subjects of programming religion.



if __name__ == '__main__':
unittest.main(verbosity=2)
0