Skip to content
This repository has been archived by the owner on May 7, 2020. It is now read-only.

fix AbstractWatchServiceTest for macOS - again #3316

Merged
merged 1 commit into from May 2, 2017

Conversation

htreu
Copy link
Contributor

@htreu htreu commented Apr 26, 2017

WatchQueueReader was "fixed" due to incorrect tests. This roles back the change and fixes the test.

Signed-off-by: Henning Treu henning.treu@telekom.de

Copy link
Contributor

@sjsf sjsf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmmm.... I just tried it:

subdirs are registered, but dirs modifications are not watched(org.eclipse.smarthome.core.service.AbstractWatchServiceTest)  Time elapsed: 10.007 sec  <<< FAILURE!
java.lang.AssertionError: Exactly 1 watch events were expected, but were: []
Expected: is <1>
     but: was <0>
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
	at org.junit.Assert.assertThat(Assert.java:865)
	at org.junit.Assert$assertThat$3.callStatic(Unknown Source)
	at org.eclipse.smarthome.core.service.AbstractWatchServiceTest$_assertFileCreateEventIsProcessed_closure11.doCall(AbstractWatchServiceTest.groovy:289)
	at org.eclipse.smarthome.core.service.AbstractWatchServiceTest$_assertFileCreateEventIsProcessed_closure11.doCall(AbstractWatchServiceTest.groovy)
	at sun.reflect.GeneratedMethodAccessor56.invoke(Unknown Source)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.codehaus.groovy.reflection.CachedMethod.invoke(CachedMethod.java:90)
	at groovy.lang.MetaMethod.doMethodInvoke(MetaMethod.java:324)
	at org.codehaus.groovy.runtime.metaclass.ClosureMetaClass.invokeMethod(ClosureMetaClass.java:278)
	at groovy.lang.MetaClassImpl.invokeMethod(MetaClassImpl.java:1016)
	at org.codehaus.groovy.runtime.callsite.PogoMetaClassSite.call(PogoMetaClassSite.java:39)
	at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCall(CallSiteArray.java:45)
	at org.codehaus.groovy.runtime.callsite.PogoMetaClassSite.call(PogoMetaClassSite.java:54)
	at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:112)
	at org.eclipse.smarthome.test.OSGiTest.waitForAssert(OSGiTest.groovy:220)
	at sun.reflect.GeneratedMethodAccessor11.invoke(Unknown Source)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.codehaus.groovy.runtime.callsite.PogoMetaMethodSite$PogoCachedMethodSiteNoUnwrap.invoke(PogoMetaMethodSite.java:187)
	at org.codehaus.groovy.runtime.callsite.PogoMetaMethodSite.callCurrent(PogoMetaMethodSite.java:56)
	at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCallCurrent(CallSiteArray.java:49)
	at org.codehaus.groovy.runtime.callsite.PogoMetaMethodSite.callCurrent(PogoMetaMethodSite.java:61)
	at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callCurrent(AbstractCallSite.java:153)
	at org.eclipse.smarthome.test.OSGiTest.waitForAssert(OSGiTest.groovy:191)
	at org.eclipse.smarthome.test.OSGiTest.waitForAssert(OSGiTest.groovy)
	at sun.reflect.GeneratedMethodAccessor10.invoke(Unknown Source)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.codehaus.groovy.runtime.callsite.PogoMetaMethodSite$PogoCachedMethodSiteNoUnwrapNoCoerce.invoke(PogoMetaMethodSite.java:207)
	at org.codehaus.groovy.runtime.callsite.PogoMetaMethodSite.callCurrent(PogoMetaMethodSite.java:56)
	at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCallCurrent(CallSiteArray.java:49)
	at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callCurrent(AbstractCallSite.java:133)
	at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callCurrent(AbstractCallSite.java:141)
	at org.eclipse.smarthome.core.service.AbstractWatchServiceTest.assertFileCreateEventIsProcessed(AbstractWatchServiceTest.groovy:289)

Did the maven build run successfully on your machine?

@kaikreuzer
Copy link
Contributor

Mine is slightly different, but also failing:

Failed tests: 
  AbstractWatchServiceTest.AbstractWatchQueueReader processWatchEvent path in root provided with correct arguments:80->assertByRelativePath:322->fullEventAssertionsByKind:335->OSGiTest.waitForAssert:-1->OSGiTest.waitForAssert:191->OSGiTest.waitForAssert:220 At least one watch event is expected
Expected: is <true>
     but: was <false>

* move watch directory creation/deletion before/after each test

Signed-off-by: Henning Treu <henning.treu@telekom.de>
@htreu
Copy link
Contributor Author

htreu commented Apr 27, 2017

I did another modification which creates/removes the root watch directory before/after each test case. This way the main build should also be stable now.

@htreu
Copy link
Contributor Author

htreu commented Apr 27, 2017

@SJKA & @kaikreuzer would be nice to get some macOS user feedback - again :-)

@kaikreuzer
Copy link
Contributor

@htreu You have asked for it :-)

The good news is:

Tests run: 210, Failures: 0, Errors: 0, Skipped: 0

The bad news is: The execution of just 8 tests takes 3 minutes (have a close look at the "Time elapsed" values):

Running org.eclipse.smarthome.core.service.AbstractWatchServiceTest
Tests run: 8, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 161.798 sec - in org.eclipse.smarthome.core.service.AbstractWatchServiceTest
AbstractWatchQueueReader processWatchEvent path in subsubdir provided with correct arguments(org.eclipse.smarthome.core.service.AbstractWatchServiceTest)  Time elapsed: 30.578 sec
subdirs are registered and modifications are watched(org.eclipse.smarthome.core.service.AbstractWatchServiceTest)  Time elapsed: 10.056 sec
AbstractWatchQueueReader processWatchEvent path in root provided with correct arguments(org.eclipse.smarthome.core.service.AbstractWatchServiceTest)  Time elapsed: 30.538 sec
same file names in root and subdir are correctly processed(org.eclipse.smarthome.core.service.AbstractWatchServiceTest)  Time elapsed: 20.015 sec
AbstractWatchQueueReader processWatchEvent path in subdir provided with correct arguments(org.eclipse.smarthome.core.service.AbstractWatchServiceTest)  Time elapsed: 30.522 sec
subdirs are not registered, but dirs modifications are watched(org.eclipse.smarthome.core.service.AbstractWatchServiceTest)  Time elapsed: 10.031 sec
subdirs are not registered and modifications are not watched(org.eclipse.smarthome.core.service.AbstractWatchServiceTest)  Time elapsed: 10.006 sec
subdirs are registered, but dirs modifications are not watched(org.eclipse.smarthome.core.service.AbstractWatchServiceTest)  Time elapsed: 20.043 sec

@sjsf
Copy link
Contributor

sjsf commented Apr 28, 2017

The bad news is: The execution of just 8 tests takes 3 minutes

Here again we are back at the discussion that Java (still) falls back to polling the file system on MacOS (see e.g. here).

I know only two ways of improving this:

  1. Use Sun's non-API SensitivityWatchEventModifier to increase the polling frequency

  2. Apparently there is a way of doing it natively: https://github.com/gjoseph/barbarywatchservice. Maybe it's worth having a look at this project or write our own implementation using the same native APIs.

Last but not least:

  1. live with it as it is.

@kaikreuzer
Copy link
Contributor

  1. skip these tests on MacOS.

@kaikreuzer
Copy link
Contributor

@htreu Which option do you prefer?

@htreu
Copy link
Contributor Author

htreu commented May 2, 2017

I would for now go with option 3 and leave it as it is. The build runs cleanly now and doing a full build&test happens not so often. The main purpose of this PR is to be able to run all test at all on macOS.

@maggu2810
Copy link
Contributor

The bad news is: The execution of just 8 tests takes 3 minutes (have a close look at the "Time elapsed" values):

skip these tests on MacOS.

I agree that such a long test time is not really nice, but skipping the test is IMHO not the intention of tests at all. 😉

So, for me option three makes sense.

But I am not a Mac user, so I leave it up to @kaikreuzer and @SJKA. But then we should state that a CI infrastructure for ESH should not be run on a Mac.

@kaikreuzer
Copy link
Contributor

Ok, so I'll just have a coffee more whenever I execute the local build ;-)

@kaikreuzer kaikreuzer merged commit deff189 into eclipse-archived:master May 2, 2017
@kaikreuzer kaikreuzer modified the milestone: 0.9.0 Jun 26, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants