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

robot.http has no effect in tests #19

Closed
pchaigno opened this issue Dec 18, 2015 · 8 comments
Closed

robot.http has no effect in tests #19

pchaigno opened this issue Dec 18, 2015 · 8 comments

Comments

@pchaigno
Copy link
Contributor

It looks like there is an issue with robot.http when using hubot-test-helper. The following script works fine with a Hubot shell but the associated test does not seem to trigger any HTTP request.

Script:

module.exports = (robot) ->
  robot.hear /(http(?:s?):\/\/(\S*))/i, (res) ->
    url = res.match[1]
    res.send "ok1: #{url}"
    robot.http(url).get() (err, response, body) ->
      res.send "ok2: #{url}"

Test:

context "user posts link", ->
    beforeEach ->
      room.user.say 'user1', 'http://google.com'

    it 'hubot posts details about the link', ->
      expect(room.messages).to.eql [
        ['user1', 'http://google.com']
        ['hubot', 'ok1: http://google.com']
        ['hubot', 'ok2: http://google.com']
      ]
@mtsmfm
Copy link
Owner

mtsmfm commented Dec 19, 2015

Hmm, I'll investigate 💫
Thank you for your report 🌠

@kengz
Copy link
Contributor

kengz commented Jan 12, 2016

Hi @pchaigno the problem is on two fronts. I ran the tests as below:

Helper = require('hubot-test-helper')
helper = new Helper('../scripts/s1.coffee')

co     = require('co')
expect = require('chai').expect

# test ping
describe 'http', ->
  beforeEach ->
    @room = helper.createRoom(httpd: false)

  # Test case
  context 'user posts link', ->
    beforeEach ->
      co =>
        yield @room.user.say 'user1', 'http://google.com'

    # response
    it 'does not expect deplayed callback', ->
      console.log @room.messages
      expect(@room.messages).to.eql [
        ['user1', 'http://google.com']
        ['hubot', 'ok1: http://google.com']
      ]

    # response
    it 'expects deplayed callback from ok2', ->
      console.log @room.messages
      expect(@room.messages).to.eql [
        ['user1', 'http://google.com']
        ['hubot', 'ok1: http://google.com']
        ['hubot', 'ok2: http://google.com']
      ]
  • Note that I included co for flow control. This is similar to issue room.user.say() and responses are not executed in series... #20 , where if you don't use co it doesn't wait for a reply (the entire thing is promise-based).
  • the room object shall be initialized properly with @room, which binds it to the co for usage. Otherwise there will be no response either.

Anyways, the first test does not expect deplayed callback passed, the second test expects deplayed callback from ok2 failed. In both cases, if you print out the @room.message it is

[ [ 'user1', 'http://google.com' ],
  [ 'hubot', 'ok1: http://google.com' ] ]

Perhaps the delayed action from ok2 isn't properly binded with the Promise object from co, so co didn't wait for the second part robot.http(url).get()... to resolve. To put it:

  • robot.http doesn't bind to the Promise of co for resolving promises in the test.

I'll leave this to @mtsmfm

@mdelagrange
Copy link
Contributor

I don't think that using co really matters in this case. Mocha is smart enough to wait on the resolution of a promise returned by a test method:

http://mochajs.org/#working-with-promises

In this case, the Promise is returned by the receive method in the test helper. So co really has no effect one way or another.

The real problem is that there is no Promise associated with the http call, which is asynchronous. I doubt there is a way to force the test to wait for the result of the HTTP call. In this case I'd use a mocking library to stub out the http call.

(In other words, your HTTP call is probably being triggered, but the test finishes before the callback is executed.)

@kengz
Copy link
Contributor

kengz commented Jan 23, 2016

@mdelagrange I agree, but there's no simple way to obtain the promise from the bot script. However since it is promise based, just like real-life interaction, the tester shall expect some time delay from the bot, although it cannot called directly (there's no handle as mentioned) when the promise resolves.

Adding that manual, expected time delay solves it, and the beforeEach body will wait for a new more seconds for the second message above to return and be added to the room.

Below is a snippet slightly modified from my earlier version that works.

Helper = require('hubot-test-helper')
helper = new Helper('../scripts/s1.coffee')

co     = require('co')
expect = require('chai').expect

# test ping
describe 'http', ->
  beforeEach ->
    @room = helper.createRoom(httpd: false)

  # Test case
  context 'user posts link', ->
    beforeEach ->
      co =>
        yield @room.user.say 'user1', 'http://google.com'
        # delay one second here
        yield new Promise((resolve, reject) -> 
            setTimeout(resolve, 1000);
          )

    # response
    it 'expects deplayed callback from ok2', ->
      console.log @room.messages
      expect(@room.messages).to.eql [
        ['user1', 'http://google.com']
        ['hubot', 'ok1: http://google.com']
        ['hubot', 'ok2: http://google.com']
      ]

passes the test: ✓ expects deplayed callback from ok2

@mtsmfm @pchaigno consider this solved :)

@kengz
Copy link
Contributor

kengz commented Jan 30, 2016

@mtsmfm in the light of this issue, can we implement a feature where sending a message will emit an event 'message sent', and so when the room handles the event by expecting a message; then when it receives the message it resolves and returns a promise?

This is more reliable than waiting an arbitrary time - I found out that during busy times on Travis it could take up to 5 seconds for a room to receive a message.

@mtsmfm
Copy link
Owner

mtsmfm commented Jan 30, 2016

Sorry for late response 😓
I'm looking for the way to test without setTimeout, but it seems that there are no way without it 😢

I found out that during busy times on Travis it could take up to 5 seconds for a room to receive a message

Thanks very useful advice 👍
I'll write it in our README in this (or next) weekend and I'll close this issue.
(But I'm not native english speaker so I'm very happy if someone create PR 😄 )

kengz added a commit to kengz/hubot-test-helper that referenced this issue Jan 30, 2016
mtsmfm added a commit that referenced this issue Feb 1, 2016
add issue #19 manual delay solution to README
@mtsmfm
Copy link
Owner

mtsmfm commented Feb 1, 2016

close via #25

@mtsmfm mtsmfm closed this as completed Feb 1, 2016
pchaigno added a commit to pchaigno/hewbot that referenced this issue Feb 15, 2016
pchaigno added a commit to pchaigno/hewbot that referenced this issue Feb 15, 2016
pchaigno added a commit to pchaigno/hewbot that referenced this issue Feb 15, 2016
@eh-am
Copy link

eh-am commented May 25, 2017

So, the best solution still is to add a manual delay?

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

5 participants