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

bpo-35730: Use known good font in IDLE test_sqeezer #11552

Closed
wants to merge 1 commit into from

Conversation

terryjreedy
Copy link
Member

@terryjreedy terryjreedy commented Jan 14, 2019

Tk guarantees that TkFixedFont exists.

https://bugs.python.org/issue35730

Tk guarantees that TkFixedFont exists.
@terryjreedy terryjreedy added type-bug An unexpected behavior, bug, or error tests Tests in the Lib/test dir needs backport to 3.7 labels Jan 14, 2019
@terryjreedy terryjreedy self-assigned this Jan 14, 2019
@vstinner
Copy link
Member

@vstinner vstinner removed the needs backport to 3.7 label just now

I did that to prevent merging the change too early. First we have to make sure that the test pass on the Gentoo buildbot.

cls.font_family = get_font_family()

@classmethod
def tearDownClass(cls):
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems unnecessary.

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably true -- at the moment. But this will be needed if root is created in setUpClass.


@classmethod
def setUpClass(cls):
cls.font_family = get_font_family()
Copy link
Contributor

Choose a reason for hiding this comment

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

This could also just be a global constant, i.e. set FIXED_WIDTH_FONT_FAMILY = get_font_family().

Copy link
Member Author

Choose a reason for hiding this comment

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

That would have to be wrapped in try...except SkipTest... to avoid skipping all non-gui tests. As it is, the current line needs the same to avoid skipping non-gui tests within SqueezerTest. But, looking ahead, I want to split SqueezerTest into SqueezerGuiTest and SqueezerNoGuiTest and move root creation into the GUI class setUpClass. (This should also make it faster.)

@taleinat
Copy link
Contributor

taleinat commented Jan 14, 2019

@terryjreedy, this is a great fix! I just added a few minor comments.

Copy link
Contributor

@eamanu eamanu left a comment

Choose a reason for hiding this comment

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

LGTM. I agree with @taleinat 's coments

@terryjreedy
Copy link
Member Author

@vstinner Removing the backport label does not prevent merging. Omitting both news and skip news, which I did intentionally, with the intention to add one later, did prevent merging, at least initially. Adding skip news turned the button green and enabled merging ;-).

Now, how does one force a particular buildbot to test a particular PR without merging?
I see a [Force] button but no place to enter a PR #.

@vstinner
Copy link
Member

@vstinner Removing the backport label does not prevent merging.

I don't want to block this PR. I want to block a backport to 3.7 until we can double check that this change fix the Gentoo buildbot on master.

@vstinner
Copy link
Member

Now, how does one force a particular buildbot to test a particular PR without merging?

Use https://devguide.python.org/buildbots/#custom-builders to test your PR on all buildbots at once.

If you only want to test on one specific buildbot, contact the buildbot worker owner who may or may not answer :-)

@taleinat
Copy link
Contributor

taleinat commented Jan 16, 2019

I've triggered a run on all buildbots via custom-builders.

EDIT: One of the relevant Gentoo builders had an error running the tests for the custom build:

Upon execvpe b'./target/bin/pythoncustom' [b'./target/bin/pythoncustom', b'-Wdefault', b'-bb', b'-E', b'-m', b'test.regrtest', b'-rwW', b'-uall', b'-j2'] in environment id 3054377756
:Traceback (most recent call last):
  File "/buildbot/venv/lib/python3.6/site-packages/twisted/internet/process.py", line 445, in _fork
    environment)
  File "/buildbot/venv/lib/python3.6/site-packages/twisted/internet/process.py", line 523, in _execChild
    os.execvpe(executable, args, environment)
  File "/usr/lib/python3.6/os.py", line 568, in execvpe
    _execvpe(file, args, env)
  File "/usr/lib/python3.6/os.py", line 583, in _execvpe
    exec_func(file, *argrest)
FileNotFoundError: [Errno 2] No such file or directory: b'./target/bin/pythoncustom'

@terryjreedy
Copy link
Member Author

terryjreedy commented Jan 16, 2019

That's annoying. Looking at https://buildbot.python.org/all/#/grid, the only normally green buildbots to turn red are the 3 Gentoo bots. The other non-refleak bot had the same error on test startup. The refleaks bot did run the test, failing with '6 not greater than 6'.
https://buildbot.python.org/all/#/builders/15/builds/12/steps/4/logs/stdio

I strongly suspect from the latter that the fix would not work on the normal Gentoo bots and that there is something peculiar about tk on the Gentoo machine. I am inclined to let this sit a bit.

In the latter, I noticed some new 'Theme Changed' messages from recent tests, including squeezer. I will look into at previous issues where I dealt with them.

@taleinat
Copy link
Contributor

I've got another idea how to make this test work properly; see PR #11585.

@terryjreedy
Copy link
Member Author

I merged the other solution, so closing this.

@terryjreedy terryjreedy deleted the fontfamily branch January 18, 2019 06:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting core review tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants