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.

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

Jump to bottom

[iOS] editing/async-clipboard/* 20 tests are flaky timeouts on iOS #191

Closed
wants to merge 0 commits into from
Closed

[iOS] editing/async-clipboard/* 20 tests are flaky timeouts on iOS #191

wants to merge 0 commits into from

Conversation

JonWBedard
Copy link
Member

@JonWBedard JonWBedard commented Mar 7, 2022 •
edited

5f5aff9

[iOS] editing/async-clipboard/* 20 tests are flaky timeouts on iOS
https://bugs.webkit.org/show_bug.cgi?id=237372
<rdar://problem/89689914 >

Reviewed by Alexey Proskuryakov.

* Tools/CISupport/kill-old-processes: Kill "Simulator" process (since killing Simulator.app
won't do anything), shutdown simulators before killing processes. Check that simulators
are shut down before continuing.


Canonical link: https://commits.webkit.org/248136@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@290905 268f45cc-cd09-0410-ab3c-d52691b4dbfc

@JonWBedard JonWBedard self-assigned this Mar 7, 2022
@JonWBedard
Copy link
Member Author

I missed @aproskuryakov's original comment on https://commits.webkit.org/220407@main when I landed it.

I was wrong to add .app to Simulator, and while I don't quite remember the specifics of that issue, it seems likely it has to do with the fact that we were killing the app before shutting down all the simulators, which was a bit silly. That should be rectified by this change.

aproskuryakov
aproskuryakov reviewed Mar 7, 2022
View reviewed changes
Tools/CISupport/kill-old-processes Outdated
@@ -165,14 +165,16 @@ def main(user=None):
# This allows to specify on the environment of the bot a list of process that should be killed.
tasksToKill = os.getenv("WEBKITBOT_TASKSTOKILL", "").split()
if sys.platform == 'darwin':
os.system("ps aux")
for task in tasksToKill + tasksToKillMac:
os.system("killall -9 -v -m " + task)
# Shutdown any simulators
if os.geteuid() == 0 and user:
os.system("sudo -u {} xcrun simctl shutdown booted".format(user))
Copy link
Contributor

Choose a reason for hiding this comment

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

This only shuts down one simulator, not all of them. Also, I think that this just starts the shutdown, and we need to wait for it to finish.

Copy link
Member Author

Choose a reason for hiding this comment

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

Will swap booted for all.

I'll put together a function that checks if any simulators are booted, but we'll need a timeout, we still want to kill all other processes if simulators are failing to shut down.

@JonWBedard JonWBedard force-pushed the eng/iOS-editingasync-clipboard-20-tests-are-flaky-timeouts-on-iOS branch from 2f135b3 to a3e0956 Compare March 7, 2022 20:46
aproskuryakov
aproskuryakov approved these changes Mar 7, 2022
View reviewed changes
Tools/CISupport/kill-old-processes Outdated
try:
simctl_output = json.loads(subprocess.check_output(
['xcrun', 'simctl', 'list', '--json'],
**(dict(encoding='utf-8') if sys else dict())
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not familiar with "if sys else", when is sys false?

Copy link
Member Author

Choose a reason for hiding this comment

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

🤦‍♂️ When I forget to make it if sys.version_info > (3, 0) else, will fix before landing

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we even need to support Python 2 still?

Copy link
Member Author

Choose a reason for hiding this comment

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

I've been dragging my feet on actually breaking Python 2 in scripts, since we've occasionally discovered regressions in our scripts when run with Python 3 and have temporarily reverted those scripts to Python 2 to resolve these regressions. I don't expect kill-old-processes to revert to Python 2, but keeping compatibility here isn't too much of a burden.

@JonWBedard JonWBedard force-pushed the eng/iOS-editingasync-clipboard-20-tests-are-flaky-timeouts-on-iOS branch from a3e0956 to 196481e Compare March 7, 2022 21:09
@JonWBedard JonWBedard closed this Mar 7, 2022
@JonWBedard JonWBedard force-pushed the eng/iOS-editingasync-clipboard-20-tests-are-flaky-timeouts-on-iOS branch from 196481e to 5f5aff9 Compare March 7, 2022 21:17
@JonWBedard
Copy link
Member Author

Landed https://commits.webkit.org/248136@main (5f5aff9)!

@JonWBedard JonWBedard deleted the eng/iOS-editingasync-clipboard-20-tests-are-flaky-timeouts-on-iOS branch March 7, 2022 21:17
@JonWBedard JonWBedard added Tools / Tests Tools in the Tools directory, build issues, test infrastructure, and bugs in test cases WebKit Nightly Build labels Mar 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aproskuryakov aproskuryakov aproskuryakov approved these changes

@whsieh whsieh Awaiting requested review from whsieh

@dewei-zhu dewei-zhu Awaiting requested review from dewei-zhu

Assignees

@JonWBedard JonWBedard

Labels
Tools / Tests Tools in the Tools directory, build issues, test infrastructure, and bugs in test cases
Projects
None yet
Milestone
No milestone
2 participants
@JonWBedard @aproskuryakov