moll / node-mitm

Intercept and mock outgoing Node.js network TCP connections and HTTP requests for testing. Intercepts and gives you a Net.Socket, Http.IncomingMessage and Http.ServerResponse to test and respond with. Super useful when testing code that hits remote servers.
Other
636 stars 48 forks source link

on('data', callback) with mitm server object #7

Closed abelnation closed 9 years ago

abelnation commented 9 years ago

I can't seem to get the on('data') event to fire for the mitm server object. Is this expected behavior?

moll commented 9 years ago

Hey,

Do you do something like the following?

mitm.on("request", function(req, res) { req.on("data", console.log) })
abelnation commented 9 years ago

Sorry, i'm using the TCP socket mocking, not http request, so using something like:

    this.mitm = Mitm()
    this.mitm.on('connect', function(server) {
            this.server = server
            this.server.on('data', function(data) {
                this.socketDataReceived = true
                console.log('...data!...')           
            }
    })
abelnation commented 9 years ago

Also tried calling on with the mitm object:

    this.mitm = Mitm()
    this.mitm.on('connect', function(server) {
            this.server = server
            this.mitm.on('data', function(data) {
                this.socketDataReceived = true
                console.log('...data!...')           
            }
    })
moll commented 9 years ago

The first approach you used should work. Just to be sure, something is definitely writing to that socket, right?

abelnation commented 9 years ago

Haha, well, that's what my test case is trying to verify. But I will write an explicit test case that opens up a socket directly to test that functionality.

moll commented 9 years ago

I have some tests doing sync IO (using socket.read), but just wrote an async one just in case:

it("must emit data on server side", function(done) {
  this.mitm.on("connection", function(s) {
    s.on("data", done.bind(null, null))
  })

  Net.connect({host: "foo"}).write("Hello")
})

That passed.

moll commented 9 years ago

Let me know if you get it working.

abelnation commented 9 years ago

Here's my test case I'm trying to verify with (let me know if the coffeescript is a problem for ya):

describe 'mitm', () ->

    # called after all `before` callbacks
    # create new logger for each spec
    beforeEach () ->
        @server = null
        @socketOpened = false
        @socketDataReceived = false
        @socketError = false
        @socketClosed = false

        @mitm = Mitm()
        @mitm.on 'connect', (server) =>
            @socketOpened = true
            @server = server
            @server.on 'data',    () =>
                console.log('...data!...')
                @socketDataReceived = true
            @server.on 'error',   () =>
                console.log('...error!...')
                @socketError = true
            @server.on 'close',   () => @socketClosed = true

    afterEach () ->
        @mitm.disable()

    it '. fires data event properly', (done) =>
        socket = require('net').createConnection(1234, '127.0.0.1')
        socket.setEncoding('utf8')
        socket.write('hello!')
        setTimeout () =>
            expect(@socketOpened).to.be.true
            expect(@socketDataReceived).to.be.true
            expect(@socketClosed).to.be.false
            done()
        , 200
moll commented 9 years ago

So, I take it that doesn't work? ^_^ Just in case, what Node version are you running?

Off-topic, but I suggest you do not use the property-access assertion method that Chai and some others incorporate. ;-) I've written a little on why asserting on property access is dangerous.

moll commented 9 years ago

If it indeed doesn't work, would you mind cding to Mitm's directory (node_modules/mitm), running npm install and then make test?

abelnation commented 9 years ago

using version 0.10.32. working to see if i can get a simple test case working

abelnation commented 9 years ago

Ah, one difference between your example and mind, was I was using the mitm.on('connect') event instead of the mitm.on('connection') event

abelnation commented 9 years ago

Ok. the eventname was def the primary issue as far as i can tell. The difference between connect and connection is not really clear from your README

moll commented 9 years ago

Great to hear you got it working eventually.

I'll take that into account. Their difference is indeed a single word — client vs server. ^_^ I can totally see how that's not entirely clear. Sorry about that. :)

abelnation commented 9 years ago

RE must vs. should, is the only motivation behind must.js that the property getter form of assertions is prone to typo errors?

moll commented 9 years ago

Not entirely, but that's one huge bad design choice whose continued existence I can't wrap my head around. A true bastardization. Like wearing a T-shirt with suit pants.

Both Chai.js and Should.js also had a few really stupid bugs (like an empty object equaling an empty array etc.) after which I had no trust in their quality. API design-wise Must.js is and will continue being simpler. It won't have chain-hacks like foo.should.deep.equal; will be consistent in its own vs inherited property checking in matchers and so on. It also has a bunch of convenience matchers that others don't and will get more once I get around to it. :-)

moll commented 9 years ago

Can I help further somehow within this issue? Feel free to close it when you feel like it. ;-)

abelnation commented 9 years ago

Thanks for all your help