Bugzilla – Full Text Bug Listing
| Summary: | [FieldAssist] Automate the manual test cases | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Susan McCourt <susan> | ||||
| Component: | UI | Assignee: | Platform UI Triaged <platform-ui-triaged> | ||||
| Status: | CLOSED WONTFIX | QA Contact: | |||||
| Severity: | normal | ||||||
| Priority: | P3 | CC: | cocoakevin, fabian.pfaff, Lars.Vogel, remy.suen | ||||
| Version: | 3.5 | ||||||
| Target Milestone: | --- | ||||||
| Hardware: | PC | ||||||
| OS: | Windows XP | ||||||
| Whiteboard: | stalebug | ||||||
| Bug Depends on: | |||||||
| Bug Blocks: | 265466 | ||||||
| Attachments: |
|
||||||
|
Description
Susan McCourt
We discussed trying to do some of this during 3.5 end-game, but I'm not assigning a milestone as this is a "nice to have" and not critical for 3.5. I think it should be done before we make any more functional changes to field assist. Bug 271339 demonstrates that it's getting harder to make changes without breaking someone. Created attachment 134971 [details] Generalize field assist tests patch v1 This patch introduces some new classes that are intended to make our testing lives a lot easier instead of copy/pasting the same code three times like we did in bug 271339. The commented out testX() method is intended to test the case described by bug 271339 comment 32. The idea is to create some abstract classes that will do the basic and some public methods to tweak the actual CPA/CACA. Then there's another batch of abstract classes that would actually extend TestCase and provide protected methods in addition to doing some simple setUp() tearDown()s. Subclasses would do the actual testing (invoke content assist, send KeyDown, check shell count, etc.) and then the last level of the hierarchy would simply return Combo- or Text-related details. Hence, the superclass can be reused for testing both Combo and Text cases. With this kind of setup, I think it should be much more straightforward for us to add more support for field assist such as StyledText (included in bug 262846) and CCombo. :) -------- Hierarchy as follows, and should make things clearer (I hope): AbstractFieldAssistWindow +ComboFieldAssistWindow ++ComboCommandFieldAssistWindow +TextFieldAssistWindow ++TextCommandFieldAssistWindow AbstractFieldAssistTestCase +AbstractContentAssistCommandAdapterTest ++ComboContentAssistCommandAdapterTest ++TextContentAssistCommandAdapterTest -------- AbstractFieldAssistWindow -abstract IControlContenteAdapter getControlContentAdapter() -abstract Contrl createFieldAssistControl(Composite) Amongst other methods, this class is a simple subclass of Window that lets subclasses define their own field assist's contents. There are also public methods for setting things like autoactivation delay and such. -------- ComboFieldAssistWindow TextFieldAssistWindow Extends the AFAW and implements the two methods to return appropriate field assist controls (Combo or Text) and the corresponding control content adapter. -------- ComboCommandFieldAssistWindow TextCommandFieldAssistWindow Extends the CFAW and TFAW and overrides the CPA construction method defined in AFAW to construct a CACA. -------- AbstractFieldAssistTestCase Defines some simple protected methods for sending low-level SWT events, setting text to the control, and such. -------- AbstractContentAssistCommandAdapterTest Extends the AFATC to provide a convenience method for executing the content assist handler through the workbench. Also implements the actual test classes. -------- ComboContentAssistCommandAdapterTest TextContentAssistCommandAdapterTest Extends ACACAT to simply construct a CCFAW and a TCFAW. The tests are defined and subsequently performed by ACACT. Remy, I'll take this one to look at getting this in for 3.6. Does this still represent the state of the art (I know you were talking to Francis at one point about other places where we fake events)... (In reply to comment #3) > Does this still > represent the state of the art (I know you were talking to Francis at one point > about other places where we fake events)... At the moment, this is what I've got that works (on my computer anyway). Francis, Eric, and I were investigating DND-related stuff actually, which is unrelated to field assist at the moment. For referencing purposes, the bug Francis opened against SWT requesting assistance is bug 263695. Fixed in HEAD >20091014. Thanks again, Remy. I changed the framework methods that fake key events to actually post an event to the display. That meant some of the test cases that faked an event and then set the content to match it didn't have to set content anymore. Posting the event allows us to test lower level constructs that require cooperation of the native control, like "propagate keys." I added some simple tests for propagate keys, autoactivate, etc. I did not get as detailed as I originally intended, simply because I don't have any more time to put into this, but this gives us a decent start and I'll ensure there are new tests added for any future fixes or new function added. Reopening. The field assist tests that I added (raw field assist, not the content assist command adapter) are failing in the nightly build. They are passing locally. Need to investigate. For the missing workbench failures, it may be worth instantiating and opening a shell and granting focus to that shell instead. It is after all not entirely inconceivable for someone to want to just run the JFace tests via a standard JUnit runner instead of PDE's JUnit runner (not having to wait for the workbench to come up saves a lot of time :)). For the other failures, given that the command adapter tests are a-okay, I think we can only assume that the event is not being posted appropriately. Perhaps using an assertion like so... assertTrue(window.getDisplay().post(event)); ...might give some insight to the problem. Note that these all pass on my XP workstation locally. Will try again on Linux/gtk+ when I get home. (In reply to comment #7) > For the missing workbench failures, it may be worth instantiating and opening a > shell and granting focus to that shell instead. It is after all not entirely > inconceivable for someone to want to just run the JFace tests via a standard > JUnit runner instead of PDE's JUnit runner (not having to wait for the > workbench to come up saves a lot of time :)). Yes, that was bogus to use the workbench there. I reran the tests with a regular launch (not workbench) and verified that those failures are gone. > > For the other failures, given that the command adapter tests are a-okay, I > think we can only assume that the event is not being posted appropriately. > Perhaps using an assertion like so... > > assertTrue(window.getDisplay().post(event)); > > ...might give some insight to the problem. I added these assertions, good idea. I also made sure we used the same display everywhere, vs. Display.getDefault(). I also added spinEventLoop() before originally counting the shells and also in the asserts for shell counts, in case there's a pending event or async that affects the initial or current count. (In reply to comment #7) > Note that these all pass on my XP workstation locally. Will try again on > Linux/gtk+ when I get home. Susan, the tests are NO GO for me locally on Linux/gtk+. The failures are intermittent but since there are so many of them, some of them fails for sure even if some others end up passing. Display's post(Event) method seems to be working in a small snippet I wrote (i.e. the listener is being notified) but I'm not getting many SWT.KeyDown events when I put print statements in ContentProposalAdapter. :( One thing I noticed is that X seems to take these key events very literally. When I first ran the tests, they completed and failed and...I noticed my computer started entering 'i' into my editor a bajillion times until I stopped it by hitting ESC. In one instance where I was stepping through the code X even crashed. After seeing this phenomena a couple of times, I eventually realized that these were the key down events that were being posted and confirmed this problem in a snippet. Posting _both_ key down and key up events stops this from happening although the tests _still_ fail so I guess this isn't really the problem. (In reply to comment #9) > Susan, the tests are NO GO for me locally on Linux/gtk+. The failures are > intermittent but since there are so many of them, some of them fails for sure > even if some others end up passing. Display's post(Event) method seems to be > working in a small snippet I wrote (i.e. the listener is being notified) but > I'm not getting many SWT.KeyDown events when I put print statements in > ContentProposalAdapter. :( hmmm...I have a Linux box I can try this on this weekend. One change I made in the abstract test case is to put focus on the control before sending the event. I'm wondering if there was a problem where no one had focus, so the events were just queued up until something did. But at this point I'm just guessing, I'll have to debug further. Paul said he get test failures locally on his workstation as well. His X didn't crash...or at least, he hasn't ran it enough times yet. :P Remy, are the content assist command adapter tests still passing on Linux? I'm going to disable the failing tests until this gets sorted out. (In reply to comment #12) > Remy, are the content assist command adapter tests still passing on Linux? I'm > going to disable the failing tests until this gets sorted out. I got some strange errors just now which claimed the content assist handler couldn't be found. Repeated attempts produced similar results...until my X server crashed. I think we should disable all of them (or at least the ones that use post(Event)) for now. disabled the tests. Here's an abridged version of an IRC chat from yesterday. <krbarnes> rcjsuen: any more luck with Display.post? <rcjsuen> krbarnes: I didn't try. Well, before I tried the junit thing I said I was going to do, I had to test something else for Susan...and in the process it brought down my X...again. So I just gave up. :| <rcjsuen> I should say I got discouraged to work on it anymore. <rcjsuen> Think I need to figure out this xfvb thing. <rcjsuen> tho dunno how that would interact with post(Event) <krbarnes> have you tried on any other machines? <krbarnes> linux machines <rcjsuen> well it fails on paul's machine too <rcjsuen> tho it didn't bring his machine down <krbarnes> is the test in CVS? <rcjsuen> krbarnes: yeah, it's in org.eclipse.ui.tests <rcjsuen> org.eclipse.jface.tests.fieldassist.FieldAssistTestSuite <krbarnes> rcjsuen: great.. I'll take a look at it today. <rcjsuen> oh and it fails on the test machines <rcjsuen> so i guess that's 3 boxes <rcjsuen> krbarnes: Thanks. <rcjsuen> krbarnes: you can report any findings on ~275393 so Susan will know too, thx <paulweb515> rcjsuen: One of the differences I thought was my GTK+ was older ... but the linux test machines won't be higher than me <Arbalest> Bug 275393 - https://bugs.eclipse.org/bugs/show_bug.cgi?id=275393 - Platform / UI / 3.5 - PC / Windows XP - REOPENED / normal / - Assignee: susan_franklin - [FieldAssist] Automate the manual test cases ------------ <krbarnes> rcjsuen: are you guys really flooding the queue with key events? <krbarnes> the test suite finished and the host eclipse has been getting key events for 5 minutes now. <krbarnes> mostly 'i' but some spaces <krbarnes> rcjsuen: if you create a long enough line in StyledText, you'll crash X. I suspect that's happening to you (and about to happen to me) <krbarnes> rcjsuen: also, IIRC while(readAndDispatch) is not sufficient to be sure that the event you posted has been processed. You should check the events until you get the one you're expecting. <krbarnes> rcjsuen: I think you should be posting a KeyUp for every KeyDown as well, though I'm not sure what the consequences are for not doing so. <rcjsuen> krbarnes: yeah i changed it to post KeyDown too (locally not on CVS) but the tests still failed <rcjsuen> krbarnes: Waiting and checking for the one that we want sounds like something worthwhile to try though. <krbarnes> I'm not sure why it was happening, but I never stopped getting events.. <rcjsuen> krbarnes: well that matches your guess last week that keydown with no keyup = *depressed key................ infinity*, no? <krbarnes> could be <rcjsuen> krbarnes: Did your system ever crash? <krbarnes> no, but it did make the machine unusable. I had to reboot <rcjsuen> lol <krbarnes> felipe says that a long enough line is StyledText will crash X though. Some old bug <rcjsuen> in my case X crashed but i was back in my ol' tty1 so i could just go $ startx again <rcjsuen> we're not testign ST tho <rcjsuen> unless you had an editor open (and had focus) <rcjsuen> and sent 'i' over there i suppose Running the code below in a JUnit test method on Linux/gtk+ seems to work as expected.
I get the following output:
>>><<<
>>>i<<<
>>>i<<<
If I comment out the rAD() call then I just get three lines of '>>><<<'. So ignoring the problem of X crashing, it seems that the posted events are not being "sent" to the right control.
------------------
final Display display = getDisplay();
Shell shell = new Shell(display);
shell.setSize(200, 50);
shell.setLayout(new FillLayout());
final Text t = new Text(shell, SWT.SINGLE);
shell.open();
System.out.println(">>>" + t.getText() + "<<<"); //$NON-NLS-1$ //$NON-NLS-2$
Event event = new Event();
event.character = 'i';
event.type = SWT.KeyDown;
display.post(event);
while (display.readAndDispatch());
System.out.println(">>>" + t.getText() + "<<<"); //$NON-NLS-1$ //$NON-NLS-2$
event = new Event();
event.character = 'i';
event.type = SWT.KeyUp;
display.post(event);
System.out.println(">>>" + t.getText() + "<<<"); //$NON-NLS-1$ //$NON-NLS-2$
shell.dispose();
looks like this isn't going to resolve for M3. Marking M4. Remy, do we need to open an SWT bug with that last snippet? (In reply to comment #17) > looks like this isn't going to resolve for M3. Marking M4. > Remy, do we need to open an SWT bug with that last snippet? Maybe, I need to talk to Kevin again tomorrow. He looked at comment 16 today and suggested that I add a while rAD() loop after the shell was opened (to ensure that the "standard" events were completed) but it seems that adding that after shell.open() just gives me three blank lines instead of the expected 'i' events. Very odd... The shell positioning tests need to use more interesting layouts than a single control in a field assist window. I totally missed a blatant bug (bug 293199) by doing my detailed testing on too simple of a case. (Of course a sanity check of the find/replace dialog is always the right thing to do! I got too comfortable with my tests). The change noted in comment 18 seems to work on and off. When it fails I see the 'i' character show up in my editor so I guess it was "missed". Adding a while loop to check that the control's content actually changes helps but is still not foolproof as it's kind of a 50-50 thing. Sometimes I will run the tests and it will just hang there with the text caret blinking at me in the while loop so I guess the key events got lost in the X server's abyss somewhere. other work taking priority over this...marking 3.6 to keep on radar. I had to disable the focus-related tests in ControlDecorationTests. It seems that I can't ensure the focus-related events are processed by the control before making the assertions, even though I spun the event loop before doing so. I must be missing something obvious. (In reply to comment #22) > I had to disable the focus-related tests in ControlDecorationTests. > It seems that I can't ensure the focus-related events are processed by the > control before making the assertions, even though I spun the event loop before > doing so. I must be missing something obvious. Clarifying previous remark. I added some ControlDecorationTests in N20091202-2000. The focus-related part of this test failed in the build, so I've disabled it for now. I'll come back to it while looking at this bug. This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. As such, we're closing this bug. If you have further information on the current state of the bug, please add it and reopen this bug. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. -- The automated Eclipse Genie. |