Upgrading dependencies in capture-server

[Christian Johansen]

Hi,

I spent some time upgrading how capture-server uses its
dependencies. I almost completed the task, but there's one change
I'm not able to do. I may have just stared myself blind, but I
need some help :) Hoping August can chip in on this.

Here's what I've done:

  - Remove buster-core (almost complete)
  - Use bane.createEventEmitter(...) in place of
    buster.extend(buster.eventEmitter.create(), ...)
  - Upgrade ramp-resources to 0.5. This requires the 0.7 branch from
    the ramp-resources repo. This is just minor API changes.
  - Use buster-node for testing to avoid the circular dependency
    between ramp-capture-server and buster.

Here's the problem:

When I try to remove the two last occurences of the old event emitter,
things start going haywire:

./lib/session-client.js
./lib/slave.js

Converting these causes timeouts in the tests, and after looking into
this problem quite a bit, I am not able to grok why. It seems that
some `this` is lost along the way, but I'm not seeing how. Could you
try to replace these and make it work?

    // In ./lib/session-client.js
    this._privateEventEmitter = buster.eventEmitter.create();
    // Should be
    this._privateEventEmitter = bane.createEventEmitter();
    // (remember to require("bane")

    // In ./lib/slave.js
    module.exports = buster.extend(buster.eventEmitter.create(), {
    // Should be
    module.exports = bane.createEventEmitter({
    // (again, require("bane")

I'm sure I've just stared myself blind and am overlooking some stupid
thing. Let me know if/when you get this working, and I'll do a few last
things.

Btw, this is available on the 0.7 branch in capture-server.

Christian

[August Lilleaas]

Semi-related, getting the "socket hang up" stuff when running tests for buster-capture-server a lot more on buster 0.7 FYI. So that's probably the only difference between our setups that caused you to get them and me not.

[Christian Johansen]

Weird. Can't think of any changes in the 0.7 modules that would cause that. Unless there's a bug somewhere.

--
MVH
Christian

[August Lilleaas]

How are you running tests now? I tried "node test/main-test.js" in buster-capture-server, no output and exit code 0.

[Christian Johansen]

Eh? `node test/main-test.js` works fine here. There's also a `run-tests.js` file:

node run-tests.js

But I can't imagine that working if running a single file doesn't. What's in your node modules? It should be this:

node_modules/
  - bane (master)
  - buster-core (this may be missing - it was removed from package.json even though I did not successfully remove it completely from the code :( Sorry about that)
  - buster-node
  - ejs
  - faye
  - htmlparser
  - lodash
  - node-uuid
  - ramp-resources (0.7 branch)
  - sinon
  - soupselect
  - when

I'm aware that the branching is really annoying, and there's no version of dev-tools to back it up. I'm sorry about this, but if you bear with me, we're very close to be off the branches. I will then provide an updated dev-tools as well.

Christian

[Christian Johansen]

Maybe the best you could do is what I did. Make a new directory with the following projects on the following branches:

buster-0.7-modules/
  - ansi-colorizer (master, https://github.com/busterjs/ansi-colorizer)
  - bane (master, https://github.com/busterjs/bane)
  - buster-capture-server (0.7)
  - buster-cli (0.7)
  - buster-configuration (0.7)
  - buster-node (master)
  - buster-sinon (0.7)
  - buster-test (0.7)
  - evented-logger (evented-logger, https://github.com/busterjs/buster-evented-logger)
  - formatio (formatio, https://github.com/busterjs/buster-format)
  - multi-glob (multi-glob, https://github.com/busterjs/buster-glob)
  - posix-argv-parser (master)
  - ramp-resources (0.7)
  - referee (master, https://github.com/busterjs/referee)
  - referee-sinon (master, https://github.com/busterjs/referee-sinon)
  - samsam (master, https://github.com/busterjs/samsam)
  - stack-filter (master, https://github.com/busterjs/stack-filter)
  - stream-logger (0.7, https://github.com/busterjs/stream-logger)

[August Lilleaas]

Have you completely stopped using buster-dev-tools? You're supposed to _not_ have buster-core, bane, and other buster modules, in node_modules, when you develop buster, in order to reuse the git repos as dependencies. It's probably not a good idea to have completely different dev environments, especially if they're (read: yours) is hard to reproduce.

Just checked out the 0.7 version and reran "buster-dev-tools deps", and "node run-tests.js" returns no output.

[Christian Johansen]

On Sun, Oct 14, 2012 at 9:12 PM, August Lilleaas <aug...@augustl.com> wrote:

Have you completely stopped using buster-dev-tools? You're supposed to _not_ have buster-core, bane, and other buster modules, in node_modules, when you develop buster, in order to reuse the git repos as dependencies. It's probably not a good idea to have completely different dev environments, especially if they're (read: yours) is hard to reproduce.

Just checked out the 0.7 version and reran "buster-dev-tools deps", and "node run-tests.js" returns no output.

Yeah, I guess I temporarily did... I have the dev-tools-like directory structure, and sometimes use NODE_PATH (like dev-tools does), but have mostly been using symlinks as a way to have an overview over the new dependency tree. The idea was that this setup wouldn't reach anyone else, because I planned on going with dev-tools again before inviting anyone to work on it. I kinda forgot when we started working on capture-server.

The main reason I haven't used dev-tools while incrementally building the new dependency tree is different branches. dev-tools would need some tinkering to support this properly, and given that this is the last round of breaking changes, I didn't think it was worth it.

Two options at this point:
1) I can give you a shell script that sets up 0.7-modules so you can look at the capture-server 0.7 things
2) I merge all feature branches, rename repos and fix dev-tools

In case of 2) we will have a master with non-functional browser testing with capture-server. Which do you prefer?
 

[August Lilleaas]

I'm actually using dev-tools currently, I just have to do the branching manually.  buster-dev-tools has a 0.7 branch with correctly named repos, so you could try using that. This would mean mkdir buster-0.7-devtools-setup, clone buster-dev-tools, run bin/buster-dev-tools pull, then checkout the correct branches in the repos that got pulled, and finally bin/buster-dev-tools deps.

[Christian Johansen]

Does that mean you got capture-server up and running?

I almost converted buster-test-cli over yesterday, so that means I can masterify ALL THE THINGS in a matter of days anyway, will be strictly using dev-tools then.

[August Lilleaas]

No, the tests are still not running (no output, exit code 0).

[Christian Johansen]

Hmm, ok. Do you have anything against me renaming/merging stuff onto master btw? That way I could get dev-tools to "just work" for the 0.7 modules.

[August Lilleaas]

Working in master is fine by me!

[Christian Johansen]

Cool, then I'll do that tonight. It'll make master not fully functional, but hopefully for a few days only.

[August Lilleaas]

Just pushed to master, we're now buster-core free in ramp.

The issue was a call to slave.removeEventListener. It has been renamed to slave.off (symmetry to slave.on for subscribing). The test run did not make this clear, as when.js swallows exceptions. More correctly, when.js makes exceptions into rejected promises, and we don't always handle rejections in our code (which is  bad, and completely our own fault).

On Fri, Oct 12, 2012 at 12:21 AM, Christian Johansen <chri...@cjohansen.no> wrote:

[Christian Johansen]

Just pushed to master, we're now buster-core free in ramp.

\o/
 

The issue was a call to slave.removeEventListener. It has been renamed to slave.off (symmetry to slave.on for subscribing). The test run did not make this clear, as when.js swallows exceptions. More correctly, when.js makes exceptions into rejected promises, and we don't always handle rejections in our code (which is  bad, and completely our own fault).

Ah, right. When is completely right to do this, we're completely stupid for not handling errors :| Are there any catch-all mechanisms in When? Would be nice if there was a concept of unhandlded errors somewhere.
 

[August Lilleaas]

Indeed, a "throw exception if no error callback is present" would be nice.

If not, I'll just go through it and handle it manually.

[Christian Johansen]

Indeed, a "throw exception if no error callback is present" would be nice.

Did you check the When docs? Pretty sure _something_ along those lines are available. I know it's been discussed at least.

C