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.

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

Conversation

csabella
Copy link
Contributor

No description provided.

@mention-bot
Copy link

@csabella, thanks for your PR! By analyzing the history of the files in this pull request, we identified @mlouielu, @terryjreedy and @kbkaiser to be potential reviewers.


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.

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

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.

@terryjreedy terryjreedy merged commit 9a02ae3 into python:master Jun 27, 2017
terryjreedy pushed a commit to terryjreedy/cpython that referenced this pull request Jun 27, 2017
terryjreedy added a commit that referenced this pull request Jun 27, 2017
#2426)

Patch by Cheryl Sabella.
(cherry picked from commit 9a02ae3)
@csabella csabella deleted the bpo24813 branch June 27, 2017 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants