-
Notifications
You must be signed in to change notification settings - Fork 175
GH-4 Added reactivation of the app after each action is generated #45
Conversation
DmitryBespalov
commented
Oct 24, 2017
- Ran Ctrl+I to reformat the code
- Split long lines that were more than 120 characters
- Added reactivation of the app after each action was made - it's not immediate, but the app is getting activated again
- Tested with Simulator: Launched the example, then minimized the app, opened Safari and waited. After some time the app was reopened.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DmitryBespalov I've checked the code on the simulator and it works fine just as you described. I think it would also work on a real device but I was not able to test it completely because when SwiftMonkey is in the background it is still randomly generating input events that get handled by the system - for me this meant that as soon as I pressed the home button, SwiftMonkey's random events opened Slack and tried to delete one message 😄 Second time it opened the Phone app and started typing some number on the keypad...
I would say this is quite dangerous if someone decides to run the tests on a real device (most likely his personal one). Maybe it would be possible to detect that the app is in the background and then stop generating actions. Then the code you wrote kicks in, brings the app to front and resume actions generation?
addAction(weight: weight) { [weak self] in | ||
guard self?.isInForeground == true else { return } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This guard statement is repeated few times in the file. Also it won't prevent triggering actions in the background if someone uses monkey.addDefaultUIAutomationActions()
to setup the tests.
I would suggest to put this code in the Monkey.addAction(...)
methods because it is a common place for both UIAutomation
and XCTestPrivate
test setup methods. Here's an example of how this could be done:
// We should definitely `typealias` the `() -> Void` to be something more verbose... like `Action`
func guardActionFromRunningInBackground(_ action: @escaping () -> Void) -> () -> Void {
return {
guard self.isInForeground else {
self.reactivateApplication()
return
}
action()
}
}
public func addAction(weight: Double, action: @escaping () -> Void) {
totalWeight += weight
randomActions.append((accumulatedWeight: totalWeight,
action: guardActionFromRunningInBackground(action)))
}
public func addAction(interval: Int, action: @escaping () -> Void) {
regularActions.append((interval: interval,
action: guardActionFromRunningInBackground(action)))
}
With this setup every action you add will only run in the foreground. In addition we'll have the reactivate app if needed logic handled here as well.
So, what do you think @DmitryBespalov ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like it.
private func reactivateApplicationIfNeeded() { | ||
if #available(iOS 9.0, *) { | ||
DispatchQueue.main.async { | ||
if XCUIApplication().state != .runningForeground{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we use the isInForeground
property here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, true
|
||
private func reactivateApplicationIfNeeded() { | ||
if #available(iOS 9.0, *) { | ||
DispatchQueue.main.async { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it needed to schedule this check to run on the main thread? If so, should we enforce that all added action run on the main thread as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of the XCUI* code must be happening on the main thread
@wojciechczerski oh by the way, the state and activate() are only available on Xcode 9! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've checked the code and it works on both Simulator and a device. I have just one comment regarding some small refactoring.
SwiftMonkey/Monkey.swift
Outdated
regularActions.append((interval: interval, action: actInForeground(action))) | ||
} | ||
|
||
typealias ActionClousre = () -> Void |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A typo: Clousre
.
Could you please move this typealias
to the top of the class? Somehow I feel it fits better there... Also since we now have a type for () -> Void
maybe it makes sense to replace all its occurrences with the new ActionClosure
? (if you do this then I guess a public
modifier will have to be added).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect! 👍