Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vows for events other then 'success' or 'error' #109

Closed
seebees opened this issue Jul 28, 2011 · 5 comments
Closed

Vows for events other then 'success' or 'error' #109

seebees opened this issue Jul 28, 2011 · 5 comments

Comments

@seebees
Copy link

seebees commented Jul 28, 2011

My understanding is that vows will listen for success and error events on a topics returned EventEmitter. It will forward the arguments from a success event to vows with one argument (?) and error events to vows with two arguments (?).

But what about events other then success or error? I don't mind writing the code, but I didn't want to do a bunch of stuff and then have someone tell me I'm crazy.

What I'm proposing is to use the event name to group the vows. Like this:

"A topic that emits many events" : {
  topic: function () {
    return new CrazyEmitter();
  },
  "end" : {
    "will be clean" : function(){}
  },
  "request" : {
    "will emit the new request" : function () {},
    "will set my class with a cached element" : function () {}
  }
}

Would output something like:

A topic that emits many events
    ✓ on request: will emit the new request
    ✓ on request: will set my class with a cached element
    ✓ on end: will be clean

Clearly the legacy behavior would be supported, so vows that do not match an event would be treated as they are currently, but you could clearly call out 'success' or 'error' if you like.

Incidentally, this would also take care of issue #76, and it might help make the callback(err, result) vs .emit( business a little clearer. If you me wanted to go really crazy I could even have a special "event" callback so you could mix returning an EventEmitter and this.callback. But maybe that's just crazy talk.

Just so we are all clear, the only way I see to do this is to cheat. I would hook EventEmitter.prototype.emit. I'm pretty sure there is a special place in hell reserved just for me, but in the meantime I would have fewer topics and more vows.

@fnl
Copy link

fnl commented Aug 25, 2011

I am having exactly this problem: I am writing a client on top of HTTP request that behaves just as the HTTP ClientRequest, so the callback receives the single response emitter. Now, vows doesn't like this at all... Just to get it to work somehow, I had to define my test functions as:

function (res, junk) {
  // test something on res...
}

I don't understand why the "callback(err, res)" pattern is hard-coded into vows functionality? Even worse, if your callbacks don't work exactly with that pattern, vows tries to take the response and parse it to JSON (yet another issue, raised by someone else)... So if your callbacks work differently, vows might start reporting errors about circular references, instead of anything even remotely sensible.

@cloudhead
Copy link

I like this idea.

@seebees
Copy link
Author

seebees commented Aug 26, 2011

all right. I'll put something together.

@seebees
Copy link
Author

seebees commented Aug 26, 2011

I have a two questions. How should I know that the sub object is an event and not another context? e.g. that "end" from my example above is an event and not simply a sub topic? I see a few options.

  1. Because "end" and "request" do not have a topic, therefore they are events.
  2. We wait until topic.on('success') fires and search through emitted events and route the results.
  3. Some kind of syntax, e.g. "[end]" that means these vows are for an event.

Inside this question is a routing question. vows uses topic.addvow() as a wrapper for topic.on('success', magic) should I extend this magic to allow for an ordering of events?

topic.addvow = function(vow, event) {
  this.on(event, magic);
}

I'm not always a fan of arbitrary syntax but, backwards compatibility... I lean a little towards something like:

"A topic that emits many events" : {
  topic: function () {
    return new CrazyEmitter();
  },
  "[request]" : {
    "will emit the new request" : function () {},
    "will set my class with a cached element" : function () {},
    "[end]" : {
      "will be clean" : function(){}
    }
  }
}

This would dictate that end should fire after request. It would also mean that I am not waiting for success at all. This means that Array.prototype.unshift.apply(ctx.topics, arguments) moves to addVow, not sure how yet, but I need to get rid of the .on('success')

@seebees
Copy link
Author

seebees commented Aug 29, 2011

Ok, I don't think that the work is done. There needs to be a few more tests written, but I wanted someone to look at it and comment.

seebees added a commit to seebees/vows that referenced this issue Nov 20, 2011
## context.js

1. added .isEvent property (to distinguish between legacy 'success' and any sub-events)
2. added property .event that is either 'success' or the named sub-event
3. the api for sub-events is a context named either 'on' or 'events' ('events' is transformed into 'on')

## vows.js

1. updated addVow to look at the vow.binding.context.event instead of 'success'
2. if the sub-event is 'error' there is no need to listen for the topics error event again
3. Hooked EventEmitter.prototype.emit so I can capture events that happen before addVow is run.
4. Updated the vow calling params so if we have a known emitted event I don't change the calling structure (i.e. add a null error)

## suite.js

1. Throw if a sub-event or 'on' context has a topic
2. Sub-events can inherit from EventEmitter
3. changed 'success' to ctx.event
4. execute new context even if the event has already fired

Added basic tests to demonstrate "Vows with sub events"

Fixes vowsjs#109
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants