Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 338529

Summary:

[working sets] Clean up WorkingSetDropAdapterTest

Product:

[Eclipse Project] JDT

Reporter:

Dani Megert <daniel_megert>

Component:

Assignee:

Raksha Vasisht <raksha.vasisht>

Status:

VERIFIED FIXED

QA Contact:

Severity:

normal

Priority:

CC:

markus.kell.r

Version:

3.7

Keywords:

test

Target Milestone:

3.7 RC1

Flags:

daniel_megert: review+

Hardware:

All

OS:

All

Whiteboard:

Attachments:

Description	Flags
Patch	daniel_megert: review-
Patch_2	none

Description Dani Megert

2011-03-01 08:12:06 EST

I20110301-0800.

WorkingSetDropAdapterTest is ugly and does not correctly create the working sets. Besides that I'd like to remove PackageExplorerPart.internalTestShowWorkingSets(IWorkingSet[]). Use the Accessor to access non public stuff.

Comment 1 Dani Megert

2011-03-01 08:53:47 EST

IMPORTANT: First fix bug 338531 and then this one.

Comment 2 Raksha Vasisht

2011-05-05 01:48:00 EDT

Created attachment 194790 [details]
Patch

Test cleaned up 
creation of working sets and code removed from PackageExplorerPart.java to the test using Accessor.

Comment 3 Raksha Vasisht

2011-05-05 01:48:24 EDT

Dani, could you pls review?

Comment 4 Dani Megert

2011-05-05 04:46:20 EDT

Comment on attachment 194790 [details]
Patch

- 'packageExplorerPart' is not a valid field name.
- I don't see the need for the second argument in 'createJavaWorkingSets'.
- The code is still ugly: AFAICS we always only put one element into the
  selection hence creating an array in all those test methods seems overkill.
  In addition we then loop over the 1 element array in 'createSelection'.

Comment 5 Raksha Vasisht

2011-05-05 05:18:05 EDT

Created attachment 194806 [details]
Patch_2

(In reply to comment #4)
> Comment on attachment 194790 [details] [diff]
> Patch

> - I don't see the need for the second argument in 'createJavaWorkingSets'.

I added it so that you can pass other arguments as well if needed. As it is now, it is not required.

> - The code is still ugly: AFAICS we always only put one element into the
>   selection hence creating an array in all those test methods seems overkill.
>   In addition we then loop over the 1 element array in 'createSelection'.

Oh didn't notice it was like that for all methods, removed the list. 

Patch_2 committed to HEAD.

Comment 6 Raksha Vasisht

2011-05-05 05:33:07 EDT

Comment 7 Dani Megert

2011-05-05 05:34:12 EDT

> Patch_2 committed to HEAD.
Take a look in HEAD for an improved version.

Comment 8 Dani Megert

2011-05-16 06:20:00 EDT

Verified in I20110512-2000 that the test is cleaned up and released into the map files.