InternalChangedEventTest fails

[bjhargrave]

Original bug ID: BZ#2407 From: Evgeni Grigorov <e.grigorov@prosyst.com> Reported version: R4 V4.3

[bjhargrave]

Comment author: Evgeni Grigorov <e.grigorov@prosyst.com>

Created attachment 199 possible fix

The patch is attached. Could you please, check it?

junit.framework.AssertionFailedError at junit.framework.Assert.fail(Assert.java:47) at junit.framework.Assert.fail(Assert.java:53) at org.osgi.test.cases.dmt.tc4.ext.junit.InternalChangedEventTest.testPostEventWithNodesToRemovedMountPoint(InternalChangedEventTest.java:246) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25) at java.lang.reflect.Method.invoke(Method.java:597) at junit.framework.TestCase.runTest(TestCase.java:164) at junit.framework.TestCase.runBare(TestCase.java:130) at junit.framework.TestResult$1.protect(TestResult.java:106) at junit.framework.TestResult.runProtected(TestResult.java:124) at junit.framework.TestResult.run(TestResult.java:109) at junit.framework.TestCase.run(TestCase.java:120) at junit.framework.TestSuite.runTest(TestSuite.java:230) at junit.framework.TestSuite.run(TestSuite.java:225) at junit.framework.TestSuite.runTest(TestSuite.java:230) at junit.framework.TestSuite.run(TestSuite.java:225) at aQute.junit.Activator.test(Activator.java:222) at aQute.junit.Activator.automatic(Activator.java:116) at aQute.junit.Activator.run(Activator.java:58)

and

junit.framework.AssertionFailedError at junit.framework.Assert.fail(Assert.java:47) at junit.framework.Assert.fail(Assert.java:53) at org.osgi.test.cases.dmt.tc4.ext.junit.InternalChangedEventTest.testPostEventWithNodesAndNewNodesToRemovedMountPoint(InternalChangedEventTest.java:263) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25) at java.lang.reflect.Method.invoke(Method.java:597) at junit.framework.TestCase.runTest(TestCase.java:164) at junit.framework.TestCase.runBare(TestCase.java:130) at junit.framework.TestResult$1.protect(TestResult.java:106) at junit.framework.TestResult.runProtected(TestResult.java:124) at junit.framework.TestResult.run(TestResult.java:109) at junit.framework.TestCase.run(TestCase.java:120) at junit.framework.TestSuite.runTest(TestSuite.java:230) at junit.framework.TestSuite.run(TestSuite.java:225) at junit.framework.TestSuite.runTest(TestSuite.java:230) at junit.framework.TestSuite.run(TestSuite.java:225) at aQute.junit.Activator.test(Activator.java:222) at aQute.junit.Activator.automatic(Activator.java:116) at aQute.junit.Activator.run(Activator.java:58)

Attached file: dmt.tc4.patch (text/plain, 5654 bytes) Description: possible fix

[bjhargrave]

Comment author: Steffen Druesedow <steffen.druesedow@telekom.de>

OK, checked it. It seems that the failing CT's expect an IllegalStateException if postEvent is invoked on a MountPoint that belongs to an unregistered plugin. This assumption is not described in the spec and therefore wrong. The only described exception is IllegalArgumentException, if the params are incorrect.

I modified the proposed fix a bit to avoid ArrayIndexOutOfBoundsExceptions and pushed it to git (hope this is OK for you Masaki ).

Assigned it to Masaki to have a final check ...

[bjhargrave]

Comment author: Steffen Druesedow <steffen.druesedow@telekom.de>

changed Component to Residential CT

[bjhargrave]

Comment author: Evgeni Grigorov <e.grigorov@prosyst.com>

OK, checked it. It seems that the failing CT's expect an IllegalStateException if postEvent is invoked on a MountPoint that belongs to an unregistered plugin. This assumption is not described in the spec and therefore wrong. The only described exception is IllegalArgumentException, if the params are incorrect.

I modified the proposed fix a bit to avoid ArrayIndexOutOfBoundsExceptions and pushed it to git (hope this is OK for you Masaki ).

Assigned it to Masaki to have a final check ...

Thanks! The additional event check improves the validation. Just a remark, the patch contains fixes in the event listeners, so ArrayIndexOutOfBoundsExceptions is not possible. Did you check the whole patch?

[bjhargrave]

Comment author: Masaki Nakano <masaki.nakano.fc@hitachi.com>

Our interpretation is that the MountPoint is not valid anymore because the plugin is unregistered, but I agree IllegalStateException is not defined for postEvent().

I think we need to clarify the behavior for unregistered mount point. In general, throwing exception immediately when API is called is better to make debugging easier.

[bjhargrave]

Comment author: Evgeni Grigorov <e.grigorov@prosyst.com>

Our interpretation is that the MountPoint is not valid anymore because the plugin is unregistered, but I agree IllegalStateException is not defined for postEvent().

I think we need to clarify the behavior for unregistered mount point. In general, throwing exception immediately when API is called is better to make debugging easier.

The specification text is: " NOTE: attempts to invoke the postEvent method on the provided MountPoint must be ignored. "

[bjhargrave]

Comment author: Masaki Nakano <masaki.nakano.fc@hitachi.com>

Thank you for pointing out the specification text.

I have checked the fix in the git. The fix is OK.

I'll change this bug as fixed.