xmppjs / xmpp.js

XMPP for JavaScript
ISC License
2.19k stars 373 forks source link

0.9.0 release broke the xmpp-plugin tests #796

Closed wichert closed 4 years ago

wichert commented 4 years ago

I get a lot of test failures now, all looking something like this:

  roster › test › get empty roster

  /Users/wichert/Crypho/xmpp-plugins/node_modules/@xmpp/client-core/lib/Client.js:12

  Rejected promise returned by test. Reason:

  TypeError {
    message: 'Cannot read property \'prototype\' of undefined',
  }

  Client.send (node_modules/@xmpp/client-core/lib/Client.js:12:27)
  IQCaller.request (node_modules/@xmpp/iq/caller.js:53:25)
  RosterPlugin.get (packages/roster/index.js:39:8)
  get (packages/roster/test.js:71:6)

To reproduce you can checkout xmpp-plugins and run make test-ci

wichert commented 4 years ago

The tests all had a setup process that looks like this:

const {context} = require('@xmpp/test')

test.beforeEach(t => {
  t.context = context()
  t.context.middleware = _middleware(t.context)
  t.context.iqCaller = _iqCaller(t.context)
  t.context.plugin = setupPubsub(t.context)
})

I can get the tests working again by replacing context() with mockClient():

const {mockClient} = require('@xmpp/test')

test.beforeEach(t => {
  t.context = mockClient()
  t.context.middleware = _middleware(t.context)
  t.context.iqCaller = _iqCaller(t.context)
  t.context.plugin = setupPubsub(t.context)
})

Is that the correct fix? There is no documentation for @xmpp/test so I'm not exactly sure what the right approach is.

sonnyp commented 4 years ago

from which version did you upgrade?

Is that the correct fix?

If it works, yes :D (also that's what xmpp.js uses now so go ahead)

There is no documentation for @xmpp/test so I'm not exactly sure what the right approach is.

@xmpp/test is kinda internal/private to xmpp.js that's why it's not documented. happy to change after the 1.0 release though.

wichert commented 4 years ago

It does indeed work.

BTW, you may be happy to hear that Crypho is now running with xmppjs from npm in production; we switched away from our fork last week.

sonnyp commented 4 years ago

@wichert nice! \o/