jarmo / RAutomation

RAutomation
MIT License
100 stars 33 forks source link

force MsUia::Window#child to instantiate an MsUia::Window as well #42

Closed leviwilson closed 11 years ago

leviwilson commented 11 years ago

When explicitly using a :ms_uia adapter in RAutomation, when accessing the Window#child method, you would get a Window object that called down into the RAutomation::Adapter::Win32::Window methods rather than a RAutomation::Adapter::MsUia::Window like the parent did. This patch forces the Window returned from Window#child to be just like its parent.

Example Usage

# assumes that RAUTOMATION_ADAPTER is NOT set to "ms_uia" (or not set at all)
window = RAutomation::Window.new :title => /Some Title/, :adapter => :ms_uia
child = window.child(:title => /About/)
# child.adapter == :ms_uia now, rather than :win32

Notes

Since SpecHelper forces the "default" adapter for RAutomation when it runs, I had to do this outside of the normal :if => SpecHelper.adapter == :ms_uia filter. The spec assumes that the RAUTOMATION_ADAPTER environment variable is not set on your machine (so that :win32 is the default).

enkessler commented 11 years ago

It looks good to me.

enkessler commented 11 years ago

And since I've got these Contributor powers just sitting around, let me go ahead and merge this.

leviwilson commented 11 years ago

jarmo commented 11 years ago

Eric, did you also run the specs? I can't run them at the moment since i'm on mobile, but just by looking at the diff it seems to me that they are failing for other adapters than ms_uia, since RAUTOMATION_ADAPTER env variable will be set by rake spec tasks.

leviwilson commented 11 years ago

The spec specifically sets it to :ms_uia, so the environment variable would not matter and the test would still be valid for Window#child. I was just saying that you would get a false positive if you had your RAUTOMATION_ADAPTER=ms_uia set already.

enkessler commented 11 years ago

Admittedly I did not run them for this change set. I was trusting that since Levi went ahead and made a test for this that he did, in fact, run it. Also, from looking at the code, the only significant change in the whole thing was some Hash merging and after looking at the other methods that it was interacting with I would say that if things did start failing as a result of this merge then it would more likely be the case that it would be the result of uncovering a bug somewhere else rather this code being buggy.

That, and I knew you could always revert it back. Owner trumps Contributor. ;)

enkessler commented 11 years ago

Addendum: some of the specs are consistently failing for me even without this change so I might not be the best one to confirm any results at the moment.

leviwilson commented 11 years ago

@enkessler which specs are failing for you? :ms_uia ones?

enkessler commented 11 years ago

Via the default rake task, it never even gets past the first adapter. I've not tried the other adapters individually yet.

spec/window_spec.rb:54 # RAutomation::Window#class_names spec/adapter/win_32/mouse_spec.rb:5 # Win32::Mouse#click spec/adapter/win_32/mouse_spec.rb:27 # Win32::Mouse#press/#release

leviwilson commented 11 years ago

spec/window_spec.rb:54 # RAutomation::Window#class_names would fail since I have added some controls to WindowsForms.exe for some of the pull requests that are currently in limbo (and haven't updated this test to reflect teh # of windows it expects to find). Not sure about the other two for Win32::Mouse.

jarmo commented 11 years ago

Right, didn't notice that explicit set of :ms_uia adapter.

I've added 3 line comments to diff.

Eric, i'm not running specs on my machine because i don't think that Levi did not run them on his - i'm running them to make sure that they're also passing on my machine and not on just his :)

leviwilson commented 11 years ago

This has all already been merged, so you're saying in a separate pull request correct?

jarmo commented 11 years ago

I fixed it in 79429e9677