Email exchanges from the bug...

In these comments, Markus says he doesn't really like the design behind the bug, but is willing to use it as a fallback if they can't get the solution they want done in time. I believe his point is that the problem is setting VM arguments properly in launch configurations will solve the assertion enabling problem along with many others. If you go read the other related bugs, you see a lot of people complaining about different aspects of VM argument setting.

------- Comment #7 From Markus Keller 2006-01-09 05:13 [reply] -------

This is another case that should be solved by a general launch templates mechanism rather than a JUnit-specific solution (see bug 41353).

If the Debug team decides they won't fix this for 3.2, then we would have to go for the solution from comment 4.

------- Comment #8 From Markus Keller 2006-01-09 11:59 [reply] -------

(In reply to comment #5)
> Is anyone on fixing this one?
> It bothers me enough I'm willing to work on a fix.

John, if you could provide a patch for the solution from comment 4, that would be nice.

I don't think we should add a checkbox on the individual launch configuration pages to enable/disable the -ea feature. It would clutter up the UI and it's hard to motivate why there should be special UI for the -ea switch but not for all the other vm arguments.

- - - - - - - - - - - - - -

My point is that many users won't necessarily understand that enabling assertions is related to VM arguments, so they'll want a feature that is more direct and higher level. I may be over-reaching at this point, but I make one more pitch for having a higher-level feature, like a checkbox on the launch configuration box.

------- Comment #9 From John Kaplan 2006-01-10 23:41 [reply] -------

OK - I will start looking into it.
I also like the solution from comment 4, but I do have an issue with only having the option in workspace preferences and not in the launch configuration pages.

The issue is that the workspace preferences page takes a fair amount of navigation to get to, and is far away from what the user sees when they start a run.

So for usability, it's likely the option will be hidden and a lot of users won't see it.

I think the fact that enabling assertions is a VM argument is an implementation detail. I didn't look at VM arguments as I was trying to figure out how to enable assertions, and from the chatter about this on the web (if you google "enable assertions eclipse" you'll see it) shows there are plenty of other users that also think this is counter-intuitive.

So, I would prefer to have a check box for defaulting the behavior per project on the preferences panel, but then have another check box on the run.../debug... dialog panels to override it per launch configuration, and to make it more manifest to the users for better usability.

But, as I always end up saying in these discussions, it ain't my code base, so I'll go with the committers decision.

- John

- - - - - - - - - - - -

At this point I do my surfing and analysis of the code that's there. Looks like it will be a fairly straightforward fix, and I'm not convinced I was right about the check boxes on the launch configurations anymore, so I'm willing to let that part go and just do the check box on the JUnit window preferences dialog.

------- Comment #10 From John Kaplan 2006-01-21 22:57 [reply] -------

Been looking at this trying to figure out a solution. I have one for the basic use case we agreed to.

I think the best design would be to initialize new launch configurations related to junit tests to be created with a default VM arg that enables assertions if the user has set the preference in the Window > Preferences... dialog. This way the VM arg is where users can see it if they look and there's no invisible creation of VM args in the background of a launch.

To implement this solution, I found 2 possible paths to create launch configurations for junit tests. One is in JUnitLaunchShortcut.createConfiguration() that is called when the user directly launches a new test. The other is in JUnitTabGroup.setDefaults() that is called when the user starts with the "Run..." or "Debug..." dialogs to create new configurations before they run something for the first time.

The JUnitLaunchShortcut.createConfiguration() change would be easy, but the JUnitTabGroup.setDefaults() looks like it will need one other workaround because the JUnit plug-in test doesn't jump through the same implementation as the JUnitTabGroup. It has a different implementation of setDefaults in AbstractPDELaunchConfigurationTabGroup, and it already messes with the VM args, so the implementation here will have to be more intelligent to avoid double-entering the VM arg.

So that's the implementation I'd go with if you guys agree. Anyone know of other issues I haven't covered? Thoughts? Yeas or Nays?

- - - - - - - - - -

In his response, Markus comes through with some good advice. I didn't know about the DebugPlugin.parseArguments method, and it came in handy.

------- Comment #11 From Markus Keller 2006-01-23 12:07 [reply] -------

John, that sounds great. I guess in the PDE JUnitTabGroup, you have to parse the existing IJavaLaunchConfigurationConstants.ATTR_VM_ARGUMENTS. That's probably done easiest and safest by calling DebugPlugin.parseArguments(..) and then searching for arguments that start with "-enableassertions" or "-ea".

------- Comment #12 From John Kaplan 2006-01-24 00:18 [reply] -------

Thanks for the tip Markus.

One more question - Since it looks like I'll be making changes in 3 packages for this, I wanted to check on which test suites to run and add new tests to.

I've already worked with jdt.ui.tests, so I know the drill there. Are tests for the junit preferences tab in the UI there, or in another package? I don't see a jdt.junit.tests, but there are a few test packages in jdt.debug.

To get the junit functionality completely tested, I'd have to set the preference to both enable and disable assertions, then run a test and either check the VM environment somehow or set up a test that throws an assertion and check that it did/didn't assert as expected.

Can you point me to any similar kind of test junit test I could use as a model, and let me know where the best place to put new tests would be? (If the test junit plug-in tests are in a different place, let me know that too since I have to make the unique changes we discussed above.)

Thanks,
- John

------- Comment #13 From Markus Keller 2006-01-24 05:16 [reply] -------

We don't have many tests for the junit plugins. The existing tests are indeed in org.eclipse.jdt.junit.tests in the jdt.ui.tests plugin.

If you want to write a test for this bug, thats great, but it's also OK without a test. I would not try to run a generated JUnit launch configuration from within the test. Just check that when the preference is enabled, the new lauch config has the -ea set.

- - - - - - - - - -

Far be it for me to judge - but did I just hear the committer for JUnit support in eclipse tell me I don't have to write tests? I am too much of a TDD programmer to let this slide, will have to rib him about this at some point.

------- Comment #14 From John Kaplan 2006-01-24 14:51 [reply] -------

OK - good enough.
I'll move ahead on this info and see how far I get.
Thanks,
- John

OK - Much time has passed, and many things have happened I need to catch up on. Have asked some questions on the bug mail and gotten good guidance from Markus. Also, my proposal for giving a talk at EclipseCon about doing contributions like this has been accepted. I'll catch up on those things in later entries.

For this one, it's the nuts and bolts of the fix. I have done a lot of surfing the eclipse code to find what I'm going to change and how. I downloaded a new integ of eclipse to work with, and got the source straight out of the PDE to do the source surfing. Grabbed a bunch of packages just to make sure I had it covered.

To avoid more of my bad experiences on my last contribution, I want to keep track of which packages I need and which files I intend to change, and get no more than that once I start messing with CVS. Note you can work with the PDE to get source to mess with, but the test packages are missing, and you need to get them directly from CVS if you're going to alter tests. So, you might as well get everything from CVS so you can keep it coordinated.

I won't go through the boring details of all the surfing and tracing I did to find the spots in the code I will need to change. Suffice it to say you have to do a lot of this when making eclipse changes. It is (as we say in Colorado) a big honkin' bunch of code, and it is factored into thousands of tiny specialized method calls. The "Search" menu and pop-up menu "Open Declaration" commands are your best friends.

For UI classes, do a text file search for text on the display. That may lead you straight to a utility class with the text as a comment, or to a preferences file with the text. From there, you can search for the preferences file constant ID or for a class that references the utility class.

For internal classes, you need to do more digging. Since assertions will be enabled through a VM arg, I searched for "vm arg" in a file text search, and got a haystack full of hits. I knew this would also have something to do with a launch configuration, so that narrowed it down quite a bit.

Once you've got a good suspect target class, set a breakpoint in the spot you think is the most likely, and fire up eclipse in a debugger to step through the code and watch it do its thing. That's the way I narrowed down what classes were involved in the changes I'll have to make. Here's the list I eventually got to:

Packages/files needed:
org.eclipse.jdt.junit
JUnitTabGroup
JUnitLaunchShortcut
JUnitPreferencePage

org.eclipse.pde.ui
AbstractPDELaunchConfigurationTabGroup
JUnitWorkbenchShortcut

(For testing)
org.eclipse.jdt.ui.tests
org.eclipse.test.performance

OK - so I grit my teeth and do a search for a bug about enabling assertions in the Eclipse bugzilla. I grit my teeth because I've done this before and it is often an exercise in frustration. The bug database is so big it hides everything like a haystack full of needles, and everybody uses obscure tribal languages in the descriptions that you hope are related to the thing you're searching for. Invariably, they're not, at least they're probably not but you're not quite sure. But then maybe that one from two searches back was really the one you wanted after all... If you try to get back to that one you can't find it because you forgot the exact keyword combination you used before and you see a bunch that look similar but not quite the same and none of them are really related to what you want so you give up.

Well, by some miracle of violated expectations, I found the exact bug I was going to enter right near the top of the query results. It had been sitting for a couple of years and no one had fixed it. It was bug #45408, titled "Enable assertions during unit tests [JUnit]", opened by Kevin Klinemeier. Here is the stack of discussion from the bug, with my offer to fix it second from the end:

Description: [reply] Opened: 2003-10-22 14:49

As I understand it, Java 1.4 assertions are disabled by default unless the vm
argument "-ea" exists. I would like very much for Eclipse to have an option to
always enable assertions while running my unit tests from the "run as... JUnit
test" menu item.

I realize that I can define a runtime configuration and specify the parameters
there, but if this were the "default", that would be very helpful.


------- Comment #1 From Dan Allen 2005-06-03 00:53 [reply] -------

Additionally, I would like to see a switch somewhere in the run dialog titled
"Enable Assertions". Having to type the -ea switch in the JVM arguments is far
too low-level for eclipse and not very user friendly. I would almost even argue
adding a "Run Java Application with Assertions" in the Run menu.


------- Comment #2 From Edward Willink 2005-06-23 13:56 [reply] -------

Surely there should be a Compiler preference?


------- Comment #3 From Andrew Scherpbier 2005-06-23 14:28 [reply] -------

(In reply to comment #2)
> Surely there should be a Compiler preference?

This is not a compiler issue so much since java 1.4 assertions are always
compiled into the class files. The issue is the (default) enabling of
assertions at runtime.


------- Comment #4 From Edward Willink 2005-10-07 09:46 [reply] -------

Indeed, not a Compiler Preference.

There should be a Workspace->Preferences->JUnit config to set -ea as a default
for all auto-generated launch configurations, and possibly for manual ones too.


------- Comment #5 From John Kaplan 2005-12-14 10:58 [reply] -------

Is anyone on fixing this one?
It bothers me enough I'm willing to work on a fix.


------- Comment #6 From Edward Willink 2005-12-14 12:05 [reply] -------

There is a slightly clumsy workaround which works fine once you've learnt it.

If you put -ea as a VM argument in a Test All launch configuration, it
works, and is inherited when you select some test(s) to redo from the
subsequent
JUnit window.

It's only when you do a Run As->JUnit Test on an IResource selection that you
lose the -ea, and get deceived into thinking everything's good till the next
Test All.

- - - - - - - - - -

This bug was assigned to Erich Gamma. You gotta figure the dude's pretty busy, so I didn't think flaming him to get on it and get his bugs fixed would be appropriate. Instead, a diplomatic e-mail might do the trick, so I wrote the following:

Fixing eclipse bugzilla #45408 (enable assertions)

Hello Erich - I am interested in submitting a fix for this bug #45408 (Enable assertions during unit tests [JUnit]).
I see it is assigned to you and didn't want to step on toes if you were actively working on it, or planning to do so in the near future.

If it's OK with you, I'd start by making sure there was a consensus for the user interaction.

My first proposal would be to have a new panel in the Window > Preferences... > Java > JUnit section that had a checkbox for "enable assertions by default for all JUnit test configurations." Then have a checkbox in each of the Run... and Debug... JUnit configuration dialog panels that says "enable assertions." That's the basics, but also it seems this check box should remain in 2-way sync with the -ea VM argument.

I'd go with the consensus if there are other opinions as to how to do it, but I'd like to start a discussion on the bug list.

Once there was a consensus I'd like to develop and submit a fix.

Hope to hear from you soon,

Thanks,
- John

- - - - - - - - - -

Erich replied:

Hi John,

Markus Keller is now taking care of the JUnit integration. I've reassigned
the bug to Markus and I agree that the discussion should continue in
bugzilla

--erich

- - - - - - - - - -

So onwards we go...

Back at Eclipse contributions again. This time it's about assertions.

This one started when I was coding along on my poker analysis program, which I do for fun. Somewhere along the way I decided to start using Java assertions, and sprinkled them throughout my code, checking input parameters, class state invariants, and all the good things assertions are supposed to check.

Then, one of my tests uncovered a bug, and in tracing through it I found that it should have triggered an assertion above where it blew up. After yelling at my code that it couldn't be doing what it was doing and why didn't it get a clue, I realized that assertions weren't enabled. I had made a tacit assumption that for testing (like when you're running JUnit) assertions would be enabled by default. Well, since they're obviously not, I searched in Eclipse help for how to enable assertions. Surprisingly, I found nothing. Then I yelled at the Eclipse help file and it didn't respond any more than my code had.

OK, so next stop on the hacker highway is google, so I searched for "enable assertions eclipse." When I first did this (which was last month - I am writing this a bit after the fact) it took a bit of digging, and most of the hits were people asking how in the name of the deity of their choice do you do this, and screaming in frustration. But a bit down the list, somebody mentions it's a VM argument, and you enable assertions by setting an "-ea" flag on the java virtual machine when you launch. Note if you do this same google search now, you get much better info, and the top 5 hits all tell you about the VM flag (including the long version "-enableassertions") right away.

OK - so thus enlightened I go to my favorite Eclipse launch configuration dialog, type in the magic "-ea" incantation, and voila, my code blows up with assertion errors all over the place and I lose a bunch of time fixing obscure bugs in old cold code I implemented months ago along with its never before activated assertions.

Allright, well, this will never happen again. I dilligently keep re-entering the "-ea" argument whenever it dissapears because I updated Eclipse or breathed on my computer wrong or whatever. It always seems to dissapear following simple rules that anyone should know but me, and then I forget and I'm no longer running with assertions and go through the evil cycle again.

So, after a few rounds of this, I decide this is enough of a pain in the rear that maybe I should try another Eclipse fix that allows you to check a check box somewhere and it remembers to enable assertions for you following rules a user who's not an Eclipse committer can understand.

Next - somebody's already entered the bug.