Bug 259787 - JUnit 4 Test Runner support
Summary: JUnit 4 Test Runner support
Status: RESOLVED FIXED
Alias: None
Product: SWTBot
Classification: Technology
Component: SWTBot (show other bugs)
Version: 2.0.0-dev   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 2.0.0   Edit
Assignee: Ketan Padegaonkar CLA
QA Contact:
URL:
Whiteboard:
Keywords: api, noteworthy
Depends on:
Blocks: 262980
  Show dependency tree
 
Reported: 2008-12-30 08:49 EST by Toby Weston CLA
Modified: 2009-01-29 17:47 EST (History)
1 user (show)

See Also:


Attachments
SWTBotRunner class and classpath changes (you'll need to reference junit-4.4.jar) (3.39 KB, patch)
2008-12-30 08:49 EST, Toby Weston CLA
KetanPadegaonkar: iplog+
Details | Diff
Adds a SWTBotSuite for JUnit 4 and tests it (5.80 KB, patch)
2009-01-23 09:38 EST, Hans Schwaebli CLA
KetanPadegaonkar: iplog+
Details | Diff
modified patch from Hans's patch (18.09 KB, patch)
2009-01-27 12:06 EST, Ketan Padegaonkar CLA
no flags Details | Diff
mylyn/context/zip (20.93 KB, application/octet-stream)
2009-01-27 12:06 EST, Ketan Padegaonkar CLA
no flags Details
Backport of patch dated 2008-12-30 08:49 -0400 to work with JUnit 4.3.1. (1.18 KB, patch)
2009-01-27 15:35 EST, Toby Weston CLA
no flags Details | Diff
Patch (15.67 KB, patch)
2009-01-27 23:00 EST, Ketan Padegaonkar CLA
no flags Details | Diff
mylyn/context/zip (10.39 KB, application/octet-stream)
2009-01-27 23:00 EST, Ketan Padegaonkar CLA
no flags Details
mylyn/context/zip (62.29 KB, application/octet-stream)
2009-01-29 00:52 EST, Ketan Padegaonkar CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Toby Weston CLA 2008-12-30 08:49:29 EST
Created attachment 121342 [details]
SWTBotRunner class and classpath changes (you'll  need to reference junit-4.4.jar)

Build ID: net.sf.swtbot.finder_1.2.0.921.jar

Steps To Reproduce:
The examples on the site show a JUnit sample extending SWTBotTestCase, I was curious if we could use a JUnit 4 mechanism. 

Please see the attached patch for a possible solution. I understand there is some dependancies with https://bugs.eclipse.org/bugs/show_bug.cgi?id=153429 although not in any detail.

The attached patch seems to work ok for me with Eclipse Version: 3.4.0 Build id: I20080617-2000 but I'm just starting to use SWTBot.

Cheers

More information:

A typical test would now look like this;


@RunWith(SWTBotRunner.class)
public class FooTest {

	private final SWTBot bot = new SWTBot();

	@BeforeClass
	public static void startTheApplication() throws TimeoutException,
			InterruptedException {
		new Thread(new Runnable() {
			public void run() {
				Main.main();
			}
		}).start();
		waitForDisplay(5, SECONDS);
	}

	@Test
	public void rightClickStartsConfiguration() throws Exception {
		bot.button("click me").click();
		bot.button("you just clicked me!").click();
	}

	private static void waitForDisplay(long timeout, TimeUnit unit) throws TimeoutException, InterruptedException {
		long endTime = System.currentTimeMillis() + unit.toMillis(timeout);
		while (System.currentTimeMillis() < endTime) {
			try {
				Display display = SWTUtils.display();
				if (display != null)
					return;
			} catch (Exception e) {
			}
			Thread.sleep(100);
		}
		throw new TimeoutException("timed out");
	}

}
Comment 1 Ketan Padegaonkar CLA 2008-12-30 11:16:05 EST
Umm... I'm unable to find a JUnit4ClassRunner in the junit4 supplied with eclipse!

Apparently this class only came about post 4.4, eclipse is using junit 4.3.1. Do you have a workaround ?

Comment 2 Ketan Padegaonkar CLA 2008-12-30 11:21:18 EST
Further investigation reveals that this JUnit4ClassRunner is deprecated and it is recommended that clients use BlockJUnit4ClassRunner instead.

The next release of eclipse is going to ship with junit 4.5. I'll package junit 4.5 with swtbot. Could you file a patch against junit 4.5 instead ?
Comment 3 Hans Schwaebli CLA 2009-01-23 09:38:57 EST
Created attachment 123525 [details]
Adds a SWTBotSuite for JUnit 4  and tests it

SWTBot can be used already with JUnit 4 according to my tests. Eclipse 3.3 includes JUnit 4.3.1.

There is no need to wait for a later JUnit version in my opinon. I suggest that SWTBot will be compatible for quite a while to Eclipse 3.3.

I have an example (see attached patch "junit4-patch.txt").

It adds the class SWTBotSuite to SWTBot. Otherwise the capturing screenshot feature would be missing when running a JUnit 4 suite.

I don't know if this is the only solution for it or if the best one, but it works.

It can be run by starting "org.eclipse.swtbot.swt.finder.junit4.AllTests" as "JUnit Test". In the launch configuration the Test Runner "JUnit 4" should be used automatically by Eclipse when the test is started.

Maybe this can still be added to SWTBot 2.0, since it obviously does not affect any other code and can provide a basic support and example for using SWTBot together with JUnit 4. If its too late for 2.0, maybe in the next version?
Comment 4 Ketan Padegaonkar CLA 2009-01-23 10:34:43 EST
My concern is mostly around the fact that junit4 tests cannot be run as part of continuous integration.

I'll be happy to put this add this support but it defeats the purpose of the test if it cannot run as part of an automated build.
Comment 5 Toby Weston CLA 2009-01-23 11:18:31 EST
(In reply to comment #3)
> It adds the class SWTBotSuite to SWTBot. Otherwise the capturing screenshot
> feature would be missing when running a JUnit 4 suite.

That's what the orginal patch addresses, capturing the screen show by adding the @RunWith annotation...
Comment 6 Toby Weston CLA 2009-01-23 11:19:12 EST
(In reply to comment #4)
> My concern is mostly around the fact that junit4 tests cannot be run as part of
> continuous integration.
> 
Can you be more specific, I run junit4 tests from CI (Hudson and TeamCity)...
Comment 7 Ketan Padegaonkar CLA 2009-01-23 11:24:37 EST
(In reply to comment #6)
> Can you be more specific, I run junit4 tests from CI (Hudson and TeamCity)...

I'm referring to eclipse plugin junit tests (bug 153429)
Comment 8 Hans Schwaebli CLA 2009-01-23 11:30:42 EST
> That's what the orginal patch addresses, capturing the screen show by adding
> the @RunWith annotation...

I overlooked it. My patch is different anyway, for example it does not use the JUnit4ClassRunner which was objected here.
Comment 9 Hans Schwaebli CLA 2009-01-23 11:47:11 EST
> My concern is mostly around the fact that junit4 tests cannot be run as part of
> continuous integration.
> 
> I'll be happy to put this add this support but it defeats the purpose of the
> test if it cannot run as part of an automated build.

I run it even outside of Eclipse and it works. It is based on Eclipse 3.3.2 target platform completely.

You use "eclipse-test-framework-3.3.zip" in your SWTBot build process. But I don't use the Eclipse Testing Framework for running the JUnit 4 tests in the continous integration.

Instead I use an approach similiar to this one: http://www.eclipse.org/articles/article.php?file=Article-PDEJUnitAntAutomation/index.html
Comment 10 Ketan Padegaonkar CLA 2009-01-23 12:10:57 EST
(In reply to comment #9)
> I run it even outside of Eclipse and it works. It is based on Eclipse 3.3.2
> target platform completely.
> 
> You use "eclipse-test-framework-3.3.zip" in your SWTBot build process. But I
> don't use the Eclipse Testing Framework for running the JUnit 4 tests in the
> continous integration.
> 
> Instead I use an approach similiar to this one:
> http://www.eclipse.org/articles/article.php?file=Article-PDEJUnitAntAutomation/index.html

Hey, this is great to hear. Can you also file a patch that contains this as well ? I'll take a look at this over the weekend. I'm really itching to move to junit 4.
Comment 11 Hans Schwaebli CLA 2009-01-23 12:19:15 EST
> Hey, this is great to hear. Can you also file a patch that contains this as
> well ? I'll take a look at this over the weekend. I'm really itching to move to
> junit 4.

Ok. But I guess this might be a too major change for 2.0 since you already begun labeling it RC1.
Comment 12 Ketan Padegaonkar CLA 2009-01-23 13:21:34 EST
Not really still awaiting to squash a few bugs to make an RC1.
Comment 13 Toby Weston CLA 2009-01-24 05:57:40 EST
ok, so it sounds like Han's patch isn't so dependant of Junit 4.5, correct?

Althought personally, I don't like using "suites" and having to provide a AllTests class to capture the tests. This might not be how J4 does things these days (I don't use suitesanymore), so if all you need to do to run Han's patch is use @RunWith(SWTBotSuite.class, I'd suggest the patches are roughly equivilant.

I didn't understand Ketan's comment as "objecting" to the JUnit4ClassRunner, rather requesting it be updated to use 4.5. I'm happy to update this if you want to give some direction here Ketan.

In terms of automated builds, I'm not a fan of having to force needless dependancies, so I run the builds by referencing junit4-xxx.jar directly from my build scripts (as well as swt/jface jars). I'd rather decouple my build process from the IDE. This is what I currently do with my project's using SWTBot.

Cheers,
Comment 14 Hans Schwaebli CLA 2009-01-25 10:25:18 EST
@Toby

I'll take a deeper look at this soon.

I think we should base it on the JUnit version which ships with Eclipse 3.3.2 and not a newer version. My patch depends on JUnit 4.3.1, the same version which is included in Eclipse 3.3.2. And SWTBot 2.0 uses version 3.3 too and does not require a later version.

The wrap method approach which you used looks elegant. But I haven't found such an API in JUnit 4.3.1, so I used the listener solution. Eclipse Ganymede (3.4) uses JUnit 4.3.1 too by the way. Version 4.5. is only used by the unfinished Eclipse 3.5.
Comment 15 Hans Schwaebli CLA 2009-01-26 06:04:45 EST
I modified SWTBot now and tested if it basically can be migrated to JUnit 4.3.1. Good news: it can. 

The ordinary JUnit tests work and also the ones with start a workbench for testing. I implemented it as I wrote before: without using the Eclipse Testing Framework.

But its quite a lot work to migrate SWTBot completely to JUnit 4 because of all those details.

I could do it completely, but it would take me longer than you. What about this approach? I migrate the a plugin which uses ordinary JUnit tests and migrate one plugin which uses a PDE Test. Then you look on it and we decide if you do the rest, or I, who does the polishing etc., okay?

Probably we can migrate SWTBot this week to JUnit 4.3.1.
Comment 16 Ketan Padegaonkar CLA 2009-01-26 12:33:44 EST
(In reply to comment #15)
> I modified SWTBot now and tested if it basically can be migrated to JUnit
> 4.3.1. Good news: it can. 
> 
> The ordinary JUnit tests work and also the ones with start a workbench for
> testing. I implemented it as I wrote before: without using the Eclipse Testing
> Framework.
> 
> But its quite a lot work to migrate SWTBot completely to JUnit 4 because of all
> those details.

This approach simulates a launch from within Eclipse and 'mocks-out' the infrastructure provided by eclipse to generate test reports.

This means that the SWTBot runtime will definitely now pull in some JDT dependencies, and *may* possibly (and I'm not sure) pull in some dependencies from PDE.

My only concern is running headless SWTBot plugin tests using the test-framework without bloating the runtime, since many users have requested that this be kept to a minimum, since the runtime may not always have JDT installed.

Comment 17 Hans Schwaebli CLA 2009-01-27 03:43:04 EST
> My only concern is running headless SWTBot plugin tests using the
> test-framework without bloating the runtime, since many users have requested
> that this be kept to a minimum, since the runtime may not always have JDT
> installed.

Well, I understand the objection, but I only know of this solution. What now?
Comment 18 Ketan Padegaonkar CLA 2009-01-27 03:50:04 EST
(In reply to comment #17)
> > My only concern is running headless SWTBot plugin tests using the
> > test-framework without bloating the runtime, since many users have requested
> > that this be kept to a minimum, since the runtime may not always have JDT
> > installed.
> 
> Well, I understand the objection, but I only know of this solution. What now?

I'll take a look at the patches in this bug, and apply them with modifications as needed. This will however be applicable only for plain SWT tests, and not for plugins, and I'll change the bug title to reflect that.

We could open another bug for the junit 4 support for plugin tests.

Hans would you be willing to migrate the existing tests to use JUnit 4, so that we can start dog-fooding our own stuff ?

I'd recommend signing up for an account on github.com and cloning the repository at http://github.com/ketan/swtbot, that way you can check in into your own repository, and file a *big* patch. Or keep filing tiny patches on this bug, your choice.
Comment 19 Hans Schwaebli CLA 2009-01-27 04:09:03 EST
Okay, great! I'll clone the repository and migrate the plain SWT JUnit 3 stuff of SWTBot to Junit 4.3.1. I can finish it this week.

Maybe the other approach can be told as an alternative (if someone asks for it), since there might be users who are accepting the side effects in order to use JUnit 4 for plugin tests. Or that topic is something for the FAQ, HOWTO or release notes?
Comment 20 Ketan Padegaonkar CLA 2009-01-27 07:51:54 EST
(In reply to comment #19)
> Okay, great! I'll clone the repository and migrate the plain SWT JUnit 3 stuff
> of SWTBot to Junit 4.3.1. I can finish it this week.
> 
> Maybe the other approach can be told as an alternative (if someone asks for
> it), since there might be users who are accepting the side effects in order to
> use JUnit 4 for plugin tests. Or that topic is something for the FAQ, HOWTO or
> release notes?

I agree, it should probably go into the FAQ, with some details and references to some bugs etc. Since you mention this, you're hereby volunteered to update the FAQ (http://wiki.eclipse.org/SWTBot/FAQ)
Comment 21 Ketan Padegaonkar CLA 2009-01-27 08:25:16 EST
So I found a bug :)

Given that AllTests needs to be annotated:
@SuiteClasses( { SWTBotSuiteTest.class, SWTBotSuiteTest1.class })

means that you'll end up with a startClient() in every Suite. This is necessary to make the suites run independently. However this will mean that it will start your application for every suite, which is bound to throw up exceptions.

Is there any way to get around this ?
Comment 22 Hans Schwaebli CLA 2009-01-27 09:11:41 EST
I'll add that information to the FAQ after I finished migrating to JUnit 4.

Concerning the problem with startClient(): Thats something for annotation "@BeforeClass" I think. If there is also a stopClient() method, annotated with @AfterClass, which closes the previously opened client(s), then the next tests can run without causing a exception.
Comment 23 Ketan Padegaonkar CLA 2009-01-27 11:18:50 EST
One more round of hacking, here's what I have, let me know if this makes both of you happy, and I'll commit this to trunk :)

public class SWTBotSuiteTest1 extends SWTBotSuiteTestBase { ... some test methods... }
public class SWTBotSuiteTest2 extends SWTBotSuiteTestBase { ... some test methods... }

note the *extends* above (I can't think of better way to get around this, so ideas are welcome)

public class SWTBotSuiteTestBase {
	private static boolean	clientStarted	= false;

// do nothing if the applicationIsRunning, else start the application
	@BeforeClass
	public static void startClient() {
		if (clientStarted)
			return;
        clientStarted = true;
        startApplicationInAnotherThread();
   }
}

And your AllTests can be written as:

@RunWith(SWTBotSuite.class)
@SuiteClasses( { SWTBotSuiteTest1.class, SWTBotSuiteTest2.class })
public class AllTests {
	public static Test suite() {
		return new JUnit4TestAdapter(AllTests.class);
	}
}
Comment 24 Hans Schwaebli CLA 2009-01-27 11:53:22 EST
(In reply to comment #23)
> One more round of hacking, here's what I have, let me know if this makes both
> of you happy, and I'll commit this to trunk :)
> 
> public class SWTBotSuiteTest1 extends SWTBotSuiteTestBase { ... some test
> methods... }
> public class SWTBotSuiteTest2 extends SWTBotSuiteTestBase { ... some test
> methods... }
> 
> note the *extends* above (I can't think of better way to get around this, so
> ideas are welcome)
> 
> public class SWTBotSuiteTestBase {
>         private static boolean  clientStarted   = false;
> 
> // do nothing if the applicationIsRunning, else start the application
>         @BeforeClass
>         public static void startClient() {
>                 if (clientStarted)
>                         return;
>         clientStarted = true;
>         startApplicationInAnotherThread();
>    }
> }
> 
> And your AllTests can be written as:
> 
> @RunWith(SWTBotSuite.class)
> @SuiteClasses( { SWTBotSuiteTest1.class, SWTBotSuiteTest2.class })
> public class AllTests {
>         public static Test suite() {
>                 return new JUnit4TestAdapter(AllTests.class);
>         }
> }
> 

If it works for the SWTBot tests it is good. I think it is not a super class suitable for every user. Because one might use the same client to be tested by various test classes, another one wants to close it and start it for each test class or even for each test method. So it depends on the requirements I think. This apprach solves the problem which you mentioned for tests, which need to start the client only once for all tests in the suite. It may not be a general solution suitable for every user's requirements (but it does not need to be this anyway).
Comment 25 Hans Schwaebli CLA 2009-01-27 11:54:09 EST
Sorry, I accidentally quoted the whole text.
Comment 26 Toby Weston CLA 2009-01-27 12:03:25 EST
Just to check my understanding and do a bit of a sense check, would the suite
approach mean the only real change is to annotate your class with the
@RunWith(SWTBotSuite.class) for individual test cases? or do you *need* to
create a suite class that captures all tests to be run.

Personally I feel suites are a bit old fashioned, I don't really see why I
should maintain a class (or @SuiteClasses annotation parameters) to list the
tests I want to run. I want to run tests either using a filter (**/*Test.java)
in Ant or by running all tests from my IDE (right clicking on a sub-folder if I
want to run a subset).

As I say, I'm checking my understanding so might well be wrong but I'd prefer
not to force users into having to write "suites".

Conceptually, does it make sense to run a test with a suite? That's kind of
what the annotation is saying; @RunWith(SWTBotSuite.class). That's kind of fine
but somehow doesn't sit right when I think it has to be run in the context of a
suite. Why do I have to run my test as part of a suite? I think I renamed my
annotation locally to something like @RunWith(CaptureScreenShotOnError.class)
which hopefully conveys what it does nicely.

Its all minor points really, and I'm playing devils advocate a little :D


Just out of interest, I usually start and stop the application under test per
test method (which broadly translates to one "acceptance test" for me, I do
this rather than once per test case but realise they can be equivalent
depending on the context of the tests) using a helper class. Just out of
interest, and to offer an alternative view, it can be found here;

http://code.google.com/p/gibble/source/browse/trunk/gibble/src/test-end-to-end/java/net/chazzwazzer/gibble/gui/UIThread.java 

an example using @Before and @After can be found here;

http://code.google.com/p/gibble/source/browse/trunk/gibble/src/test-end-to-end/java/net/chazzwazzer/gibble/gui/dialogs/ConfigurationWindowVisualTest.java
Comment 27 Ketan Padegaonkar CLA 2009-01-27 12:06:05 EST
Created attachment 123895 [details]
modified patch from Hans's patch

I'm attaching a patch with some code that makes the above snippet work with Hans' patch.
Comment 28 Ketan Padegaonkar CLA 2009-01-27 12:06:10 EST
Created attachment 123896 [details]
mylyn/context/zip
Comment 29 Ketan Padegaonkar CLA 2009-01-27 12:20:12 EST
(In reply to comment #24)
> If it works for the SWTBot tests it is good. I think it is not a super class
> suitable for every user. Because one might use the same client to be tested by
> various test classes, another one wants to close it and start it for each test
> class or even for each test method. So it depends on the requirements I think.
> This apprach solves the problem which you mentioned for tests, which need to
> start the client only once for all tests in the suite. It may not be a general
> solution suitable for every user's requirements (but it does not need to be
> this anyway).

You are bang on target on this one.
Comment 30 Ketan Padegaonkar CLA 2009-01-27 13:21:38 EST
(In reply to comment #26)
> Just to check my understanding and do a bit of a sense check, would the suite
> approach mean the only real change is to annotate your class with the
> @RunWith(SWTBotSuite.class) for individual test cases? or do you *need* to
> create a suite class that captures all tests to be run.

Seems it's time for me to hit the bed. I'm overlooking too many things, and completely missed the point both of you are trying to make.

Hans wants to make a runnable for a Suite, which I agree with Toby, is quite old-school.

Toby wants to make a runner for a TestCase, which is basically 'porting' the @RunWith from the suite level to a @RunWith on a testcase level(one which is not supported on 4.4, yet.)

Hans I'll apply your patch.

Toby, would it be possible to backport this behavior to work with JUnit 4.4 ? I don't see moving to 4.5 anytime until eclipse 3.5 comes out.
Comment 31 Ketan Padegaonkar CLA 2009-01-27 13:30:08 EST
(In reply to comment #30)
> Toby, would it be possible to backport this behavior to work with JUnit 4.4 ? I
> don't see moving to 4.5 anytime until eclipse 3.5 comes out.

To correct, eclipse ships with JUnit 4.3.1, the next eclipse is going to ship with JUnit 4.5. So we're going to have to backport to JUnit 4.3.1
Comment 32 Hans Schwaebli CLA 2009-01-27 14:28:01 EST
@Toby

I prefer to define suites in the code and not in the build scripts. There are suites which cannot be collected by folder/package conventions, for example smoke test suites, which might contain "arbitrary" tests from different packages.

Another example of the advantage of a suite is that you can comment tests out of it. Then it does not matter if it is run from a script or within Eclipse (single source princip): the same tests are executed.

Or if a test class is not finished, but comitted, it is not run if it is not added to the suite. But if someone simply runs all tests in a package it will be run although it is not meant to be run yet.

However I agree that a a user should not be forced to use suites.
Comment 33 Toby Weston CLA 2009-01-27 15:35:55 EST
Created attachment 123941 [details]
Backport of patch dated 2008-12-30 08:49 -0400 to work with JUnit 4.3.1.
Comment 34 Toby Weston CLA 2009-01-27 15:41:52 EST
> (In reply to comment #30)
> > Toby, would it be possible to backport this ... ?

Done, see attachment. I used the notifier mechanism as per Hans patch but extend TestClassRunner rather than Suite. Its a direct update to my orginal patch so probably doesn't reflect the latest screencapture code in SWTBotTestCase.

Just checked the name, don't know why but the class name ends with "Using" (I blame random typing!). Didn't mean to do this so the class name should be CaptureScreenShotOnFailure so in a test reads like

@RunWith(CaptureScreenShotOnFailure.class)

From my perspective, I'd be happy with either solution going in. For my projects, I'm actually using the JUnit 4.4 version of CaptureScreenShotOnFailure stored along with my source. I'd probably continue to use this as I'm on 4.4. ta 


Comment 35 Toby Weston CLA 2009-01-27 16:05:48 EST
Hi Hans,

If suites work for you, that's great! I thought it might be interesting though to put a few comments from my experience in line...

(In reply to comment #32)
> @Toby
> 
> I prefer to define suites in the code and not in the build scripts. There are
> suites which cannot be collected by folder/package conventions, for example
> smoke test suites, which might contain "arbitrary" tests from different
> packages.

I've not experienced this yet, I find that I can structure my tests to avoid it. For example, my end-to-end tests are in one folder, my unit tests in another. In previous projects I've used the postfix *VisualTest.java to represent tests that fire up a gui to distinguish them. A similar pattern people often do is name abstract tests *TestCase.java and then filter these out in the Ant scripts.

> 
> Another example of the advantage of a suite is that you can comment tests out
> of it. Then it does not matter if it is run from a script or within Eclipse
> (single source princip): the same tests are executed.
> 

I'm a bit of a purest here so prefer to run *all* tests before committing usually via Ant. Its force of habit and when I'm test driving my code, I'll run the specific test just from the IDE then lean on the Ant/Maven/whatever scripts before committing. So I don't really find myself wanting to comment out / exclude tests ad-hoc. I'd also be worried that the commented out suite or an incomplete suite gets checked in. How do you ensure that your suite stays up to date with your folders? Don't you have to add every test you write to the suite? and educate new contributers? can you enforce it in an automated way?

> Or if a test class is not finished, but committed, it is not run if it is not
> added to the suite. But if someone simply runs all tests in a package it will
> be run although it is not meant to be run yet.

Again, I might be too optimistic here but why check in an incomplete test? In the rare cases I need to do something like ignoring a test, I use @Ignore in J4 or prefix a J3 test method with "dont" (so it looks like "dontTestSomething"). However, I would think twice before committing.

> However I agree that a a user should not be forced to use suites.

I'm not clear if your patch forces that or not I'm afraid... but I'm glad we agree in principle :D

Cheers

Comment 36 Hans Schwaebli CLA 2009-01-27 17:07:31 EST
> I'm not clear if your patch forces that or not I'm afraid... but I'm glad we
> agree in principle :D

A quote from Groucho Marx: "Those are my principles, and if you don't like them... well, I have others." ;-)

> I used the notifier mechanism as per Hans patch 

The capture method is called twice in my implementation with the failure listener. I discovered it yesterday and didn't yet examine why. Maybe its because I use two nested suites. This is just a minor performance issue.
Comment 37 Ketan Padegaonkar CLA 2009-01-27 20:59:13 EST
(In reply to comment #36)
> The capture method is called twice in my implementation with the failure
> listener. I discovered it yesterday and didn't yet examine why. Maybe its
> because I use two nested suites. This is just a minor performance issue.

Your patch keeps adding the listener for every test run, which means that the 10th test will have 10 listeners, and therefore 10 screenshots :)
Comment 38 Ketan Padegaonkar CLA 2009-01-27 22:55:04 EST
Toby the patch used internal API that was removed in JUnit 4.5. In order to make everyone happy, I move to junit 4.5, SWTBot will bundle junit 4.5 to ensure that existing clients don't break.

Let me know if both of you are comfortable and I'll check this patch in.

Here's how your tests will look like:

@RunWith(FooApplicationTestClassRunner.class)
public class SWTBotSuiteTest1 {
  @Test	public void test1() {}
  // note that we do not extend or start the application, etc.
  // most magic happens in FooApplicationTestClassRunner (below)
}

Here's how your suites will look like:
@RunWith(Suite.class)
@SuiteClasses( { SWTBotSuiteTest1.class, SWTBotSuiteRunnerTest.class })
public class AllTests {
	public static Test suite() {
		return new JUnit4TestAdapter(AllTests.class);
	}
}

The application runner is now:
public class FooApplicationTestClassRunner extends SWTBotJunit4ClassRunner {

  public void startApplication() {
    AddressBook.main(null); // call your app's main here
  }

// If clients wish to stop the application after every test execution as is Toby's case here:
  public void run(RunNotifier notifier) {
    super.run(notifier);
    // do something to stop the application
  }
}

Comment 39 Ketan Padegaonkar CLA 2009-01-27 23:00:38 EST
Created attachment 123991 [details]
Patch

the said patch
Comment 40 Ketan Padegaonkar CLA 2009-01-27 23:00:42 EST
Created attachment 123992 [details]
mylyn/context/zip
Comment 41 Hans Schwaebli CLA 2009-01-28 03:34:00 EST
Thats good I think. I didn't got the idea because I was thinking SWTBot goes along with Eclipse 3.3.x entirely. But the JUnit 4.5 plugin does not seem to depend on anything, so it can be used even in 3.3.x as it seems.

The main benefit for me is that with JUnit 4.5 I can use the addon "JExample" (http://smallwiki.unibe.ch/jexample/) which adds support for test dependencies to JUnit: When test A depends test B and B fails then A is not run because it won't be sucessful anyway.
Comment 42 Toby Weston CLA 2009-01-28 04:37:28 EST
Hey Folks,

I see there being a couple of distinct ideas here

1. being able to capture a screenshot on failure
2. starting / stopping an application in a test context

So, I would suggest they're separated in SWTBotJunit4ClassRunner, for example, if I want just the screenshot for a test I may not want the start application feature. Also, I see the start/stop feature as quiet specific to an particular app (rightly or wrongly).

So for 1. I would just have a simple runner and use it with @RunWith. 

For 2. I actually prefer to be explicit about the UI thread (see http://pequenoperro.blogspot.com/2008/12/be-explicit-about-ui-thread-in-swt.html), and so would manage the start/stop within my own test base class. In fact, the way I've done this is not to extend at all, being explicit about the UI thread means delgating the application startup to a known thread but still manage that thread from the test class. I think changing from @Before/After to @BeforeClass/@AfterClass should be all that's required to change from per test to per test class.


Its interesting to see how your waitForDisplayToAppear method is implemented / how SWTBot generally holds a display which it can reference. I've been using the default display to achieve the same thing (making sure its the only display used in my app).

I think its also interesting to note that when running tests from eclipse, the stop application is actually achieved by the eclipse test runner calling system.exit! So, when not having a stopApplication type method is actually relying on this which may not be sufficient if graceful shutdown of the application is required... my solution (links in comment #26) works for me.
Comment 43 Ketan Padegaonkar CLA 2009-01-28 06:48:19 EST
(In reply to comment #42)
> For 2. I actually prefer to be explicit about the UI thread (see
> http://pequenoperro.blogspot.com/2008/12/be-explicit-about-ui-thread-in-swt.html),
> and so would manage the start/stop within my own test base class. In fact, the
> way I've done this is not to extend at all, being explicit about the UI thread
> means delgating the application startup to a known thread but still manage that
> thread from the test class. I think changing from @Before/After to
> @BeforeClass/@AfterClass should be all that's required to change from per test
> to per test class.

In which case the SWTBotJunit4ClassRunner can be made a concrete class with default implementation that does nothing, and clients can override to achieve what you mention in point 1.

I really need to make a lot of people happy by not exposing the threading model, while developers may perhaps prefer that, most poeple using SWTBot are not developers and will just not 'get' it.

Let's arrive at a concensus, if it means two SWTBotJunit4ClassRunners to make the two groups happy, then so be it.
Comment 44 Toby Weston CLA 2009-01-28 08:37:56 EST
> Let's arrive at a concensus, if it means two SWTBotJunit4ClassRunners to make
> the two groups happy, then so be it.

I'm happy with any of the approaches really, as it stands now I am able to use my own classes to control threading and capture screen shots. I wasn't really complaining before just offering another view.

So feel free to go along which ever path you feel suits the majority of users :)


> In which case the SWTBotJunit4ClassRunner can be made a concrete class with
> default implementation that does nothing, and clients can override to achieve
> what you mention in point 1. 

Last comment: Why not have SWTBotJunit4ClassRunner *not* implement the ApplicationRunner interface at all - let it do just the job of capturing screen shots (renaming it appropriatly) and have another thing which implements ApplicationRunner *and* uses the SWTBotJunit4ClassRunner (using @RunWith)? Clients can sub-class that as the start of their tests. Doesn't this decouple those two ideals yet still get you what you were aiming for?

> I really need to make a lot of people happy by not exposing the threading
> model, while developers may perhaps prefer that, most poeple using SWTBot are
> not developers and will just not 'get' it.

Out of interest, who do you see as the majority of users? 
Comment 45 Hans Schwaebli CLA 2009-01-28 13:41:32 EST
I forked and cloned your SWTBot repository and applied your patch: https://bugs.eclipse.org/bugs/attachment.cgi?id=123991

I copied "org.junit4_4.5.0.v20080911" into the target platform plugins folder after materializing workspace.

Now some questions:
1. Do you want me to delete the suites (AllTests etc.) when I migrate this to JUnit 4.5? I won't be offended if you want them deleted, I just need to know for migration.
2. I've seen you don't specified a minimum version in the MANIFEST.MF file for JUnit4 although 4.5 is required as a minimum. Do you agree to require Junit 4.5 specifically? Example: Require-Bundle: org.junit4;bundle-version="4.5.0"
3. Is there no Synchronize view for GIT? It is so nice to review what I changed before I commit it. This is possible in CVS and SVN. I didn't find that feature for GIT.
4. Do you want JUnit 4.5 plugin to be in "org.eclipse.swtbot.releng/externals/plugins" folder together with its source plugin?
5. As I understood, we use JUnit 4.5 and you only want the simple JUnit tests to be migrated but not the plugin tests. Is there anything other to consider for test migration?

I will build SWTBot and run all SWTBot tests before and after the migration to see if the result is the same and the migration is correct.
Comment 46 Ketan Padegaonkar CLA 2009-01-28 23:38:19 EST
(In reply to comment #45)
> I forked and cloned your SWTBot repository and applied your patch:
> https://bugs.eclipse.org/bugs/attachment.cgi?id=123991
> 
> I copied "org.junit4_4.5.0.v20080911" into the target platform plugins folder
> after materializing workspace.
> 
> Now some questions:
> 1. Do you want me to delete the suites (AllTests etc.) when I migrate this to
> JUnit 4.5? I won't be offended if you want them deleted, I just need to know
> for migration.

-1. AllTests are available at a package level so I can execute tests in a particular package, it is useful for running tests for localized changes.

> 2. I've seen you don't specified a minimum version in the MANIFEST.MF file for
> JUnit4 although 4.5 is required as a minimum. Do you agree to require Junit 4.5
> specifically? Example: Require-Bundle: org.junit4;bundle-version="4.5.0"

+1

> 3. Is there no Synchronize view for GIT? It is so nice to review what I changed
> before I commit it. This is possible in CVS and SVN. I didn't find that feature
> for GIT.

Limitation of the GIT tooling, I'm using the command line, here :) If you're on windows I'd recommend one of the better GUI tools for this.

> 4. Do you want JUnit 4.5 plugin to be in
> "org.eclipse.swtbot.releng/externals/plugins" folder together with its source
> plugin?

Don't worry, I'll add this in immediately, I've got approval to use JUnit 4.5. You should get these changes when you do a pull.

> 5. As I understood, we use JUnit 4.5 and you only want the simple JUnit tests
> to be migrated but not the plugin tests. Is there anything other to consider
> for test migration?

Nothing else.
Comment 47 Ketan Padegaonkar CLA 2009-01-29 00:51:59 EST
- Created a and SWTBotJunit4ClassRunner that captures screenshot, and SWTBotApplicationLauncherClassRunner (that in addition, launches applications).
- Updated manifest to use JUnit 4.5.

Pushed patch in trunk.

Hans, please open up another bug to track migration of existing tests.
Comment 48 Ketan Padegaonkar CLA 2009-01-29 00:52:09 EST
Created attachment 124122 [details]
mylyn/context/zip
Comment 49 Ketan Padegaonkar CLA 2009-01-29 00:54:26 EST
Comment on attachment 121342 [details]
SWTBotRunner class and classpath changes (you'll  need to reference junit-4.4.jar)

Welcome to the SWTBot hall of fame!
Comment 50 Ketan Padegaonkar CLA 2009-01-29 00:54:33 EST
Comment on attachment 123525 [details]
Adds a SWTBotSuite for JUnit 4  and tests it

Welcome to the SWTBot hall of fame!
Comment 51 Hans Schwaebli CLA 2009-01-29 17:47:10 EST
> Hans, please open up another bug to track migration of existing tests.

I did now: https://bugs.eclipse.org/bugs/show_bug.cgi?id=262980