Hi, Thanks for investigating test failure and very nice explanation about the problem. I've just quickly read your proposed code and it looks really fine. It'll be really much appreciated If you make a pullrequest :-)
Thanks, moon On Thu, Apr 16, 2015 at 5:14 AM Eugene Morozov <[email protected]> wrote: > Hi! > > Right after git clone today (my head commit is > 1df116fe4a44fc5d3712817ea725a872d9d6402e) I’ve tried to build zeppelin, but > zeppelin-interpreter fails. I digged into it and discovered the following. > The proposal’s commit can be seen in my github branch: > https://github.com/fathersson/incubator-zeppelin/commit/0e5a2f4d958fe35e88d9c2bb57f05ae019a3d5d7 > > Some insights of what I discovered. > There is an RemoteInterpreterEventPoller, which is a Thread and while it’s > running it constantly gets and release clients, so with a little of debug > messages it looks like: > before getClient: 0 > after getClient: 1 > before release: 1 > after release: 0 > before getClient: 0 > after getClient: 1 > before release: 1 > after release: 0 > > But then, in the test RemoteInterpreterProcessTest.testClientFactory() > after it creates RemoteInterpreterProcess and references it (thus starts > that poller), it gets another client. So, after this call > RemoteInterpreterProcess would contain either 1 or 2 clients at the same > time. It just matter of timing if the following will pass or not: > assertEquals(1, rip.getNumActiveClient()); > > Just out of the code itself, I’d propose to set new > RemoteInterpreterEventPoller(interpreterGroup, this) from outside and then > start it in the reference method. That helps to test it as it’s possible to > provide any Thread (including mock(theReal.class)) that would do nothing, > but not sure if it’s the best - don’t yet understand the code good enough. > See the link above. > > But of what I see, the whole test basically checks if GenericObjectPool > (commons-pool2:2.3) is actually working. I bet apache has pretty thorough > testing for it. > > Eugene Morozov > [email protected]
