eclipse-ocl / org.eclipse.ocl

Eclipse Public License 2.0
0 stars 0 forks source link

[tests] JUnit tests are difficult to run #346

Closed eclipse-ocl-bot closed 1 month ago

eclipse-ocl-bot commented 1 month ago

| --- | --- | | Bugzilla Link | 254919 | | Status | CLOSED FIXED | | Importance | P3 normal | | Reported | Nov 11, 2008 12:28 EDT | | Modified | May 27, 2011 02:48 EDT | | Reporter | Ed Willink |

Description

Created attachment 117567 (attachment deleted)\ JUnit test launch config and support

The attached resolves a variety of problems I had when trying to run the Ecore and UML JUnit tests. The main problem was that the initialisation for running as a JUnit test was missing from the AbstractTestSuite classes.

The new launch configurations allow UML or Ecore tests to be run as plugins or standalone, with or without the System.out.println console output.

The default polarity of the System.out debug is on for compatibility. Now that the framework is stable, perhaps off would be a better default,\ and launch configurations offering debug are perhaps redundant. A single residual output to System.err remains; surely this should be a fail?

The standalone invocations rely on properties to identify each plug-in location. These are automatically set by the launch configurations. If invoked by Run As..., the plugin tests work (until heap is exhausted), the standalone tests instruct to use the launch configuration.

The UML test_emptyCollectionType_196972 can now be (re-)invoked in isolation; it no longer requires a preceding test to load the OCL Standard Library.

eclipse-ocl-bot commented 1 month ago

By Christian Damus on Nov 12, 2008 13:06

I'm not sure that I understand the point. It isn't difficult to run these tests. Simply select the AllTests class, and take the "Run As -> JUnit Plug-in Test" action in the context menu. The launch configuration thus created doesn't need any additional set-up, and doesn't need to be shared.

What is the problem in the debug output on the command-line? It's easy enough to ignore, especially when Eclipse provides such a tidy tree-structured view of the results.

For execution of the entire test suite in a stand-alone environment, the launch configuration in the org.eclipse.ocl.standalone.tests project sets everything up automatically, computing locations of stuff in the Ant script. I have even committed a work-around for the ExecutorService.shutdown() SecurityException so that the tests run cleanly. Sure, this runs all three tests suites and takes about a minute to complete, but it makes a good last check before committing code, that stand-alone execution isn't broken. For ad hoc runs, the JUnit Plug-in Test works well.

eclipse-ocl-bot commented 1 month ago

By Ed Willink on Nov 13, 2008 03:15

Created attachment 117739 (attachment deleted)\ JUnit test Launches and Memory leakage fixes

I'm not sure that I understand the point. It isn't difficult to run these tests. Simply select the AllTests class, and take the "Run As -> JUnit Plug-in Test" action in the context menu. The launch configuration thus created doesn't need any additional set-up, and doesn't need to be shared.

That would have been good to know. I expected "Run As -> JUnit Test" to work for the src folder in the absence of any other launching clues.

I wasted a lot of time trying to find out how the property was defined and assuming that test.xml provided it via its reference to org.eclipse.test/library.xml. Learning about org.eclipse.test and its friends of course contributed nothing.

What is the problem in the debug output on the command-line? It's easy enough to ignore, especially when Eclipse provides such a tidy tree-structured view of the results.

I find it discourteous to throw debug output out. In my layout the console view keeps popping to the front obscuring JUnit's tidy tree-structured view.


I spent more time on this because I wanted to check that changing to the BacktrackingParser didn't break anything. Not knowing about AllTests my earlier launch configs were running everything three times, and so memory leakage was exagerated. Even using AllTests the launch ran out of memory on my 1.25G laptop, so I needed to find out whether it was the tests or the parser that was causing so much leakage.

The attached therefore fixes further issues.

tearDown() nulls far more variables than before, in particular a fair number of suites weren't nulling 'ocl'. A couple of Ecore suites now null fruitPackage after change. I gave up fixing this on the UML suites and just nulled fruitPackage always. I also added some missing debug Start Finish to some suites that didn't inherit AbstractTestSuite; the debug was very useful while running with some construct/finalize instrumentation on ResourceImpl, AbstractTypeResolver, AbstractBasicEnvironment. Only one Environment and Parser remains at the end of a test run now; there seem to be a few extra http://oclenv.uml resources. The Ecore tests will now run in a 16MB JVM. The UML tests will at least run once in a fresh default size JVM.


Surely the test functionality should be shared between Ecore and UML bindings so that the binding-specific AbstractTestSuite sets up the binding-specific context in which to invoke the binding-neutral test functionality? The binding-neutral instance would then be home to all working variables so that nulling just the instance would release all the memory that the derived TestCase might otherwise lock-in.

eclipse-ocl-bot commented 1 month ago

By Christian Damus on Nov 18, 2008 10:33

(In reply to comment #2)

Created an attachment (id=117739) [details] JUnit test Launches and Memory leakage fixes

I'm not sure that I understand the point. It isn't difficult to run these tests. Simply select the AllTests class, and take the "Run As -> JUnit Plug-in Test" action in the context menu. The launch configuration thus created doesn't need any additional set-up, and doesn't need to be shared.

That would have been good to know. I expected "Run As -> JUnit Test" to work for the src folder in the absence of any other launching clues.

I'm sorry you didn't know that, but the AllTests class as a collector of all of the tests in the suite is pretty much a standard at Eclipse (or, perhaps, that's something that came from my IBM team?). In any case, a simple "how do I run the tests?" question in the newsgroup would have answered that.

I wasted a lot of time trying to find out how the property was defined and assuming that test.xml provided it via its reference to org.eclipse.test/library.xml. Learning about org.eclipse.test and its friends of course contributed nothing.

Why waste time? Just ask.

What is the problem in the debug output on the command-line? It's easy enough to ignore, especially when Eclipse provides such a tidy tree-structured view of the results.

I find it discourteous to throw debug output out. In my layout the console view keeps popping to the front obscuring JUnit's tidy tree-structured view.

That's why the Console view has, in the action bar, options to prevent it from popping up whenever new output is appended.

Also, your launch configuration can choose not to allocate a console at all. It's a simple toggle in the Common tab. Need print-lines? Turn it on. Don't need them? Turn it off.


I spent more time on this because I wanted to check that changing to the BacktrackingParser didn't break anything. Not knowing about AllTests my earlier launch configs were running everything three times, and so memory leakage was exagerated. Even using AllTests the launch ran out of memory on my 1.25G laptop, so I needed to find out whether it was the tests or the parser that was causing so much leakage.

The attached therefore fixes further issues.

tearDown() nulls far more variables than before, in particular a fair number of suites weren't nulling 'ocl'. A couple of Ecore suites now null fruitPackage after change. I gave up fixing this on the UML suites and just nulled fruitPackage always. I also added some missing debug Start Finish to some

I think that will slow the tests down considerably?

suites that didn't inherit AbstractTestSuite; the debug was very useful while running with some construct/finalize instrumentation on ResourceImpl, AbstractTypeResolver, AbstractBasicEnvironment. Only one Environment and Parser remains at the end of a test run now; there seem to be a few extra http://oclenv.uml resources. The Ecore tests will now run in a 16MB JVM. The UML tests will at least run once in a fresh default size JVM.

Hmm ... this sounds pretty good.


Surely the test functionality should be shared between Ecore and UML bindings so that the binding-specific AbstractTestSuite sets up the binding-specific context in which to invoke the binding-neutral test functionality? The

Yes, it would be nice. Refactoring the tests just never could be a priority. I'll try to make time to look at your patch. Thanks for making the effort!

binding-neutral instance would then be home to all working variables so that nulling just the instance would release all the memory that the derived TestCase might otherwise lock-in.

eclipse-ocl-bot commented 1 month ago

By Ed Willink on Jun 28, 2009 04:55

I'll start working on this first.

If we're going to be rationalisaing APIs and testing two different OCL versions, we will find life a lot easier if tests don;t need needless duplication.

I anticipate that the duplicated AbstractTestSuite for Ecore and UML tests will migrate to inherit a massively generic AbstractTestSuite <...> similar to Environment. There will therefore be a new org.eclipse.ocl.tests plugin that provides the neutral functionality for org.eclipse.ocl.ecore.tests and org.eclipse.ocl.uml.tests.

eclipse-ocl-bot commented 1 month ago

By Laurent Goubet on Jun 29, 2009 10:23

Ed,

I've had unit tests lying in a workspace for a good while yet haven't yet proposed a patch. These tests are related to bug 267223 : there are a fair number of incompatibilities between the EvaluationVisitorImpl implementation and the specification. I aimed at highlighting all of those I found through 95-99% coverage of "visitOperationCallExp" (my original goal is to be able to refactor this monster of a method, yet we need it fully tested before it can be done).

These tests were relying on few of the methods of AbstractTestSuite. Do you intend on keeping the API the same (I can easily cope with added genericity, removed methods might be a tad more difficult :)).

eclipse-ocl-bot commented 1 month ago

By Ed Willink on Jun 29, 2009 13:05

The intention is to change things as little as possible. I don't to rewrite (and test) all the tests.

First try genericising AbstractTestSuite was a failure. Every 'fruit' xxx variable needed to become a getXXX.

Next try will be to change all tests to inherit either EcoreTestContext or UMLTestContext which derive the very generic AbstractTestContext which will NOT derive from TestCase.

This should ensure that model elements are not locked in by JUnit tests locked in by the GUI. UML tests leak like crazy, if a new fruit model is created each time to avoid the problem that some tests modify the model under test.

eclipse-ocl-bot commented 1 month ago

By Ed Willink on Sep 14, 2009 03:08

Created attachment 147073 Patch to introduce tearDown and other consistencies

Attached is the first stage in migrating the JUnit tests to a shared framework. This stage imposes compliance with a consistent framework on the tests. This will allow the next stage to share an abstract framework for Ecore and UML tests. This will then allow tests to be written in binding-independent style.

A CheckedTestSuite class is used to enforce use of AbstractTestSuite for all tests.

Test names are defined by the suite constructor rather than the test case eliminating constructor and suite() methods per test case.

tearDown is now performed reflectively by the AbstractTestSuite avoiding the opportunity for memory leaks through non-nulling of working fields. tearDown() is not needed in test cases. A tearDown_xxx method may be provided if the default assignment of null is inappropriate to tearDown xxx.

Working statics have been eliminated reducing inter-test interactions.

The global EPackage registry is not modified.

Modification of the Fruit package is detected and must be indicated in the test by setting expectModified=true;

Some missing build warnings are reinstated.

OCLTestsTearDown.patch

eclipse-ocl-bot commented 1 month ago

By Laurent Goubet on Oct 02, 2009 10:17

Seems all good to me and can be committed as is as far as I'm concerned. One question though : why did you change all instance variables from private to package visibility? I could see no real use in increasing these visibilities.

eclipse-ocl-bot commented 1 month ago

By Ed Willink on Oct 02, 2009 10:45

In order to use Java reflection to null out fields during tearDown the field has to be visible to the inherited functionality. private fields are correctly invisible, hence package.

I didn't like releasing the visibility but the tests had so many leaks that this is the only way of ensuring that future tests do not introduce new leaks.

eclipse-ocl-bot commented 1 month ago

By Laurent Goubet on Oct 02, 2009 10:57

Ed,

I didn't pay enough attention at the exact code used by the teardown. I only asked since I didn't see the point though : private or package visibility, that doesn't really make a difference in test code ;).

+1 for the whole patch. This really speeds up the tests and a profiler shows way less memory usage :).

eclipse-ocl-bot commented 1 month ago

By Ed Willink on Oct 07, 2009 16:41

Committed to HEAD.

Next for extraction of common functionality to o.e.o.tests and migration of at least one test file to demonstrate Ecore/UMLO independence.

eclipse-ocl-bot commented 1 month ago

By Ed Willink on Oct 08, 2009 14:23

Created attachment 149152 (attachment deleted)\ New generic tests plugin

Attached is a new plugin, which the next patch exploits to share code and tests.

Includes shared bug 291310 functionality.

eclipse-ocl-bot commented 1 month ago

By Ed Willink on Oct 08, 2009 14:34

Created attachment 149155 (attachment deleted)\ Patch to exploit new shared tests plugin

This is the next stage.

Shared test setup is migrated to GenericTestSuite, from which EcoreTestSuite and UMLTestSuite derive as the basis for self-standing tests.

Shared functionality is enabled by TestReflection from which EcoreTestReflection and UMLTestReflection derive adding substantialy to the two UMLReflectionImpl.

The two KeywordsTest files are replaced by GenericKeywordsTest that contains the bulk of the code and the very thin EcoreKeywordsTest and UMLKeywordsTest that derive to define all the generic parameters. UMLKeywordsTest shows how additional tests for UML only are supported.

This satisfies my original goal of enabling binding-independent tests to be run, and should allow Laurent to write his new tests more flexibly.

At some point all the other existing tests need careful migration to exploit both sets.

Many of the tests use the Fruint meta-model, which once flexibly supported will reside in GenericFruitTestSuite from which the existing two AbstractTestSuite classes currently derive. Promotion of this setup has yet to be done.

In order to avoid an API problem there is a horrible UMLReflectionExporter class that lives in o.e.o.uml in order to export the package private class. When environments are sorted out we should make this public from an internal package the same as the Ecore variant and discover why it cannot have a static instance.

In order to make the reflective tearDown work all tests without extra code in test classes, the tests must be in the o.e.o.tests. I suugest that we migrate test classes there once they have been made binding-independent and comprise am erge of Laurent new tests and Christian's old ones.

eclipse-ocl-bot commented 1 month ago

By Laurent Goubet on Oct 13, 2009 06:10

Ed,

That's another lenghty patch; would have been easier to review if you hadn't mixed in this the removal of some "NON-NLS" tags in favor of the @suppressWarning("nls") annotation. It's a good thing you did it, but it adds way too much noise to the AbstractTestSuites for me to be sure I haven't left out some of the actual changes.

First remark, not looking at the code itself : none of the two launch configs (ecore and UML) passes anymore. All ecore tests fail in (failure) "junit.framework.AssertionFailedError: 'org.eclipse.ocl.ecore.tests' property not defined; use the launch configuration to define it". And worse, all UML tests fail in (error) "java.lang.IllegalAccessError: class org.eclipse.ocl.uml.UMLReflectionExporter cannot access its superclass org.eclipse.ocl.uml.UMLReflectionImpl".

I am not that happy with the fact we're now forced to define a property in the launch config when trying to launch standalone tests. Since we know the tests depend on org.eclipse.runtime, and you already added much uses of reflection, why not use reflection all the way? Namely, replace in the two\ "AbstractTestSuite#initFruitPackage()"\ the line\ "URI uri = getTestModelURI("/model/OCLTest.uml");"\ with\ URI uri = null;\ try {\ URL modelURL = FileLocator.resolve(getClass().getResource("/model/OCLTest.ecore")); //$NON-NLS-1$\ uri = URI.createURI(modelURL.toString());\ } catch (IOException e) {\ fail("Couldn't locate model/OCLTest.ecore in test plugin " + staticReflection.getTestPlugInId()); //$NON-NLS-1$\ }

Which totally removes the need of indicating the plugin's location as a property of the launch configuration. Of course, this would be better in its own "getTestModelURI" method so that it can be reused by other tests needing the loading of a model, but that method needs be in the AbstractTestSuite so that "class.getResource(...)" resolves to the accurate location.

You created UMLKeywordsTest and EcoreKeywordsTest in org.eclipse.ocl.tests . I'd rather we keep the name of the package as it was so that "uml" and "ecore" tests are distinguished "by package".

eclipse-ocl-bot commented 1 month ago

By Ed Willink on Oct 31, 2009 17:23

Hmm. I'm sure I replied to this a couple of weeks ago.

The failure of the tests to run must be because your workspace is not synchronized/rebuilt.

The properties have always been required for standalone operation. Your example using FileLocator only works when Eclipse is running. Unfortunately Java provides no mechanism to navigate sideways from the root package so the xx/model folder cannot be accessed wrt xx/bin/.... For QVTd test plugins I put test models at\ xx/src/..../models to solve this.

Removing the "Ecore"/"UML" package prefix is the better compromise. It has the only minor drawback that derived tests that add fields must declare them public; not a problem for tests.

eclipse-ocl-bot commented 1 month ago

By Ed Willink on Nov 01, 2009 14:31

Created attachment 151028 Fix for plugin usage

Found the problem. You didn't make clear that you were using the plugin rather than standalone usages. The plugin has a missing org.eclipse.core.runtime. Fixed in the new attachment.

See next replacement for explanation of Saved folder.

:compression: org.eclipse.ocl.tests.zip

eclipse-ocl-bot commented 1 month ago

By Ed Willink on Nov 01, 2009 14:42

Created attachment 151031 Simplified patch

Changing NLS's is a good idea, but not as we go; review is too hard. NLS changes removed. Let's do the NLS's in a separate patch that does nothing else.

Again to simplify, I've backed out the KeywordsTest migration to shared code. The changes therefore use the framework without changing any tests.

The Saved folder of the shared tests plugin demonstrates shared testing; next round of patches.

I've moved UMLReflectionImpl to an internal package with public access allowing test usage in the same way as the Ecore binding does. This avoids the UMLReflectionExporter bodge.

I've also removed the API nature from the test plugins.

:notepad_spiral: Bug254919.patch

eclipse-ocl-bot commented 1 month ago

By Ed Willink on Nov 19, 2009 05:00

Ping.

Laurent. Do you have any plans to review this submission?

eclipse-ocl-bot commented 1 month ago

By Laurent Goubet on Nov 19, 2009 07:37

Ed,

Sorry for the delay, didn't realize there was a new patch attached here. I'm quite busy with other projects and won't be able to review this patch before the end of the week, yet I'll try and do it first thing on monday.

Laurent

eclipse-ocl-bot commented 1 month ago

By Laurent Goubet on Nov 23, 2009 05:22

Ed,

I might be a little delayed in reviewing this ... for some reason Eclipse refuses to apply any of these patches (or should I say, it sees only conflicts and I cannot merge with confidence).

Are the patches up to date with the CVS version? Or should I use an older branch/date before trying to apply the patches?

eclipse-ocl-bot commented 1 month ago

By Ed Willink on Nov 25, 2009 15:39

The patches contain the CVS versions to which they apply. I find Notepad2 good for viewing patch files.

The conflicts are on the UTF-8 patches that you applied two weeks after this patch was ready for review. If you revert the two copies of AbstractTestSuite the patches then apply and you can add the checkForUTF8 method back again.

eclipse-ocl-bot commented 1 month ago

By Laurent Goubet on Nov 26, 2009 04:18

Thanks for the update, anyways, I reverted the sources to what they were when you created the patch and started reviewing from there :).

Once again sorry for delaying this; the patch seems fine to me. +1 for the commit.

eclipse-ocl-bot commented 1 month ago

By Ed Willink on Nov 26, 2009 15:45

Thanks Laurent.

It is now possible to share unit tests between Ecore and UML bindings. The Saved folder in the plugin patch has a demonstration.

Further progress in test-specific bugs.

eclipse-ocl-bot commented 1 month ago

By Anthony Hunter on Dec 08, 2009 11:57

Hi Team,

When I bring OCL HEAD into my workspace, there is a new plug-in org.eclipse.ocl.tests but this does not exist in the PSF or MAP files in the ocl.releng project.

Can you confirm? The new bundle was added by this bug I believe.

eclipse-ocl-bot commented 1 month ago

By Ed Willink on Dec 08, 2009 16:52

Confirmed, the ocl.tests is a growing body of shared Ecore/UML2 test functionality.

OCL releng is now in org.eclipse.ocl/releng rather than org.eclipse.ocl.releng.

The PSF is now on www allowing access from the PSF URL from the project page.

eclipse-ocl-bot commented 1 month ago

By Ed Willink on May 27, 2011 02:48

Closing after over 18 months in resolved state.