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

Correct flakiness in macOS CI #2637

Merged
merged 3 commits into from
Jun 11, 2024
Merged

Conversation

freakboy3742
Copy link
Member

@freakboy3742 freakboy3742 commented Jun 11, 2024

Corrects the intermittent failure seen in macOS CI with the test_secondary_window_with_content test.

The underlying cause of the crash appears to be an issue with removal of constraints; since the test involves the addition of content in a window and then rapidly deleting that window, it's at least conceptually consistent that the issue is related to ObjC reference counting on the very-short-lived window.

The problem can't be reproduced with 100% reliability, so it's difficult to confirm with 100% reliability that the problem has been resolved; but I've re-run the macOS x86-64 test multiple times with this fix in place, and it doesn't seem to be occurring any more.

This isn't a very satisfying fix - this sort of cleanup shouldn't be required; but until beeware/rubicon-objc#256 is addressed, I think it's about as good as we're going to get.

Also includes the "network fix" suggested in the discussion for #2632.

Fixes #2631.
Fixes #2632.

PR Checklist:

  • All new features have been tested
  • All new features have been documented
  • I have read the CONTRIBUTING.md file
  • I will abide by the code of conduct

@freakboy3742 freakboy3742 marked this pull request as ready for review June 11, 2024 04:15
# Github runners seem to have intermittent connectivity issues.
# See https://github.com/beeware/toga/issues/2632
- name: Tune GitHub-hosted runner network
uses: smorimoto/tune-github-hosted-runner-network@v1
Copy link
Member

Choose a reason for hiding this comment

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

This user providing this Action doesn't seem like they are likely to turn nefarious at some point...but pinning just v1 means we'll automatically start running any changes they merge. That said, the README says they plan to make no changes, so even pinning it to a specific commit hash should be fine. Throwing this out there as, at least, an observation since this is all pretty small risk.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fair; I'll update the pin to 1.0.0.

Copy link
Member

@rmartin16 rmartin16 left a comment

Choose a reason for hiding this comment

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

i'll definitely be happy if this resolves the issues because half to three-fourths of my CI runs were failing on one of these 🤞🏼

@rmartin16
Copy link
Member

testbed linux...

tests/widgets/test_selection.py::test_item_titles Fatal Python error: Aborted

:(

@freakboy3742
Copy link
Member Author

testbed linux...

Dammit CI... can you hold it together for 30 minutes?

I guess there's another problem to resolve... that's the first time I've seen this one, though. For the purposes of this PR, I'll rerun and pretend it never happened... :-)

@freakboy3742 freakboy3742 merged commit 1ae8563 into beeware:main Jun 11, 2024
31 of 34 checks passed
@freakboy3742 freakboy3742 deleted the ci-flaky branch June 11, 2024 04:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants