jonathanpenn / ui-auto-monkey

UI AutoMonkey is a simple stress testing script for iOS applications that runs in UI Automation and Instruments. Grass fed. Free range.
http://cocoamanifest.net
MIT License
1.47k stars 249 forks source link

Converted to dynamic object to be easily #imported and configured #5

Closed AliSoftware closed 10 years ago

AliSoftware commented 10 years ago

Hi,

As I liked your script and wanted to use it on multiple projects without having to copy/paste it each time, I though being able to #import "UIAutoMonkey.js" and then configure and run it would be nice.

With your version you have to change the config object inside your source to tweak the configuration. And methods dedicated to "Events" are not clearly isolated from the rest.

My suggestion is to make your script more generic so that we now can simply #import "UIAutoMonkey" at the top of our Instruments UIAutomation script, then create a new monkey, configure it and release the monkey at the end, without the need to copy/paster then edit the original source but simply import it and configure it separately

#import "UIAutoMonkey.js"

// Configure the monkey: use the default configuration but a bit tweaked
monkey = new UIAutoMonkey()
monkey.config.numberOfEvents = 1000;
monkey.config.screenshotInterval = 5;
// Of course, you can also override the default config completely with your own, using `monkey.config = { … }` instead

// Configure some custom events if needed
monkey.config.eventWeights.customEvent1 = 300;
monkey.allEvents.customEvent1 = function() { … }

// Release the monkey!
monkey.RELEASE_THE_MONKEY();

I'm not a master at javascript so feel free to correct my adaptation if needed. Let me know what you think.

AliSoftware commented 10 years ago

I realize I forgot to update the README.md, don't forget to update the documentation as now to customize an event you simply add a method, with the same name as the event and no more …Event suffix, to monkey.allEvents instead.

jonathanpenn commented 10 years ago

This is a fantastic idea!

I have one concern, though. Why did you change the name of all the event triggering methods to remove the "Event" suffix? From what I could tell, it seemed like a style and not a technical decision on you part. Let me know if I missed something about that.

The reason I named these methods to end in "Event" was to help JavaScript newcomers out or people who aren't developers by trade (like QA professionals). In my experience teaching with this tool, it was easier for them to reason about. I would prefer to keep it that way to help this tool be more generally accessible. Please let me know if there was a more fundamental reason for the change that I missed.

jonathanpenn commented 10 years ago

DOH! I can't read. I see what you did with the allEvents dictionary. Nice work! I'll pull it in.

AliSoftware commented 10 years ago

Thx for merging this :beers:

I have one concern, though. Why did you change the name of all the event triggering methods to remove the "Event" suffix?

It seems to me to be nicer to isolate all the events in an allEvents object to avoid mixing main methods of your UIAutoMonkey with method dedicated to the monkey events like tap & such. Here all the event functions are grouped and isolated in a nice object and won't risk any collision with any of your other internal methods. And they can thus have the same name as the one used in the eventWeights hash and drop the Eventsuffix, leading in some cohesion too :+1:

Seems more clear to me than having to add the Event suffix which is not necessary once the functions are isolated in a dedicated allEvents object that only contains events and nothing more :wink:


:warning: NOTE: Don't forget to update the README.md that I forgot to edit, especially to explain the new way to add custom events and customize the monkey config.

AliSoftware commented 10 years ago

DOH! I can't read. I see what you did with the allEvents dictionary. Nice work! I'll pull it in.

Yep the GitHub diff is a bit messy, due to a lot of lines being exactly the same except for whitespace / indentation, so it seems like a lot of modifications whereas it is quite simple :wink: I bet if you use a diff a tool which ignores whitespaces, you would see this intermediate allEvents object right away instead of it being drown in the middle of all those diff lines :smile:

AliSoftware commented 10 years ago

:thought_balloon: :bulb: We could also imagine an alternate way to add custom events, that some may find easier as it would do all in one call:

UIAutoMonkey.prototype.addEvent(name, weight, code) = function() {
    this.config.eventWeights[name] = weight;
    this.allEvents[name] = code;
};

// Usage
monkey.addEvent('myevent', 200, function() {
    ...
});
jonathanpenn commented 10 years ago

I think the way you set it up now is just fine. I updated the README. Should be good to go. Thanks!