Skip to content
This repository has been archived by the owner on Jul 27, 2022. It is now read-only.

Locating a widget providing one arbitrary ancestor, the widgets index and its type #11

Open
GoogleCodeExporter opened this issue May 4, 2015 · 14 comments

Comments

@GoogleCodeExporter
Copy link

This is a feature request for a new Locator, see: 
https://groups.google.com/forum/?fromgroups=#!topic/windowtester-pro/-V7MnZjiXeA
[1-25]

I am looking for a built-in way to to locate a widget with a specific ancestor.
I cannot access the direct parent though, which is a sub-type of SWT Composite, 
and for several reasons I don't want to use a NamedWidgetLocator.
Also, there is an arbitrary number of widgets of that type in the active 
context.

To solve this, I implemented a locator that works just like the 
SWTWidgetLocator with this constructor:
SWTWidgetLocator(Class<?> cls, int index, SWTWidgetLocator parent)
But instead of parent only, it also works with the grantparent or any other 
ancestor that can be wrapped in an ISWTWidgetReference, as well.

Additionally to solving the problem above, this locator can also lead to 
smaller and more robust tests because stacking several layers of 
SWTWidgetLocators to locate a single child-Composite is no longer necessary.

My custom locator is full of "Discouraged access"-warnings, so I hope that it 
will be included in the next windowtester release.
I tested it with several widgets, with and without index, but if something 
important is missing or the patch is not in line with your codinge guidelines, 
please let me know and I'll fix it.
Also, if there is an alternative solution to my problem, please let me know.

Cheers
Max

Original issue reported on code.google.com by [email protected] on 21 Aug 2012 at 11:37

Attachments:

@GoogleCodeExporter
Copy link
Author

I added a simple JUnit test and fixed an issue with the indexed 
ContainedInLocator.
I hope the test case will make it easier to understand this Locator and to get 
this issue accepted. :)

Original comment by [email protected] on 14 Sep 2012 at 12:28

Attachments:

@GoogleCodeExporter
Copy link
Author

My comment in the windowtester google group:
https://groups.google.com/d/msg/windowtester-pro/-V7MnZjiXeA/cnw7H0Q3WBQJ

Original comment by [email protected] on 12 Nov 2012 at 11:08

  • Changed state: Accepted

@GoogleCodeExporter
Copy link
Author

Changed the patch according to Freds proposal in above comment.

Original comment by [email protected] on 21 Nov 2012 at 10:11

Attachments:

@GoogleCodeExporter
Copy link
Author

I just noticed that I accidentally added an unnecessary change to 
SWTMatcherBuilder to above patch. To fix this you can either exclude said class 
when applying the patch, or use the fixed version I just attached instead. 
Sorry for the trouble.

Original comment by [email protected] on 21 Nov 2012 at 6:29

Attachments:

@GoogleCodeExporter
Copy link
Author

Hi Max,
Thanks for reworking the patch and sorry again for the long delay.
I revised your patch a little bit. Here is what I changed:
-removed UNSPECIFIED_INDEX and used UNASSIGNED instead
-moved up the containedIn methods closer to the "in" methods
-removed unnecessary code from DynamicCompositeStacksTestShell
-some minor whitespace changes

Hopefully this is the last iteration before I can commit your changes.

@Keerti: Please review my patch and let me know if I can commit it.

Regards,

Fred

Original comment by [email protected] on 5 Dec 2012 at 2:17

  • Changed state: Started

Attachments:

@GoogleCodeExporter
Copy link
Author

Hi Fred,
great to see some progress on this issue!
I hope Keerti can review the patch soon.

Best Regards
Max

Original comment by [email protected] on 10 Dec 2012 at 11:14

@GoogleCodeExporter
Copy link
Author

Patch looks good.

@Fred, go ahead and commit it.

Original comment by [email protected] on 17 Dec 2012 at 5:48

@GoogleCodeExporter
Copy link
Author

Original comment by [email protected] on 29 Dec 2012 at 9:52

  • Changed state: Fixed

@GoogleCodeExporter
Copy link
Author

Unfortunately this patch introduced a serious issue with locators that use an 
index to specify a widget.

Please find attached a bugfix. It reverts the findAll() method to work as 
before and only use the new behavior when the ".containedIn()"  locator is used.
The bugfix also includes a regression test for this issue.

@Keerti: Please review the bugfix and let me know if I can commit it.

Original comment by [email protected] on 31 Jul 2013 at 10:34

Attachments:

@GoogleCodeExporter
Copy link
Author

@Fred, thanks for fixing this. 
May I ask what the issue with indexed locators was?

Original comment by [email protected] on 7 Aug 2013 at 7:22

@GoogleCodeExporter
Copy link
Author

@Max, indexed locators where just not found.
You can easily reproduce the issue with the test case in the patch.

I'm not sure why this was discovered so late, but I guess nobody ran a test 
case that  really covered this. I setup an environment where I can now reliably 
run tests and hopefully find such regressions faster.

Original comment by [email protected] on 7 Aug 2013 at 12:27

@GoogleCodeExporter
Copy link
Author

Hi Fred so i have ran in that issue too. I toke your latest git version 
"tycho_e4" and there this problem was still there. I needed to place " || 
getParentInfo() != null" right after the line "if(getAncestorInfo()", i think 
it was just forgotten, so maybe you can fix this too, it is a one liner so i 
have no patch ready for you.

Original comment by [email protected] on 19 Sep 2014 at 12:44

@GoogleCodeExporter
Copy link
Author

Hi Falk,
The original patch and the bugfix are included in the tycho_e4 branch.
Can you add another test to the ContainedInLocatorTest class, so that I can 
reproduce the issue that you're still seeing? It would be great if you could 
submit this as a pull request on Github, but a patch is fine too.

Regards,

Fred

Original comment by [email protected] on 23 Sep 2014 at 10:14

@GoogleCodeExporter
Copy link
Author

I did a mistake, after further testing i discovered, that everything is alright 
with the condition. 

BR,
Falk

Original comment by [email protected] on 29 Sep 2014 at 9:51

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

1 participant