Use chai and sinon from npm module

[trajano]

Thanks for submitting a PR! We'll try to look at it as soon as possible.

If you are adding significant new content, please take a moment to include an update to the changelog in your PR.

Use this field to describe your pull request.

Rather than practicalmeteor:chai, use the npm versions. This allows us to
ensure we can get the latest versions possible of the libraries and allows
us to do tests using other test packages.

See practicalmeteor/meteor-mocha#23 and https://github.com/xolvio/meteor-jasmine/issues/334#issuecomment-208725856 for other reasons.

[apollo-cla]

@trajano: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

[tmeasday]

Thanks @trajano! This seems like a good idea -- could I ask that you send the equivalent PR to https://github.com/meteor/todos so we can double check that it works?

[tmeasday]

@trajano any chance of that PR?

[trajano]

Sorry no, RL work took priority. Plus there's a few errors after I upgraded meteor on the boilerplate code that I need to address first.

[abernix]

Any updates on this @trajano ?

[abernix]

@trajano No worries if you're still busy, but just checking back on this! Did you encounter any insurmountable problems with switching to this? Seems like a reasonable idea overall.

[trajano]

I don't recall having any problems when I did it back then, but Meteor has been evolving since I created the PR. No guarantees, but if I get some free time today I'll take a quick look.

[trajano]

DO NOT MERGE YET. I still have to make sure things work, I just rebased it for now.

[trajano]

Hmm like I said I haven't played with Meteor for a while, I just tried to do meteor test --driver-package practicalmeteor:mocha on Windows and got

D:\m\todos>meteor npm install
...

D:\m\todos>meteor run
... works welll ... ctrl-C

D:\m\todos>meteor test --driver-package practicalmeteor:mocha
C:\Users\trajano\AppData\Local\.meteor\packages\meteor-tool\1.4.2_3\mt-os.windows.x86_32\dev_bundle\lib\node_modules\meteor-promise\promise_server.js:190
      throw error;
      ^

Error: EPERM: operation not permitted, symlink 'C:\Users\trajano\AppData\Local\.meteor\packages\meteor-tool\1.4.2_3\mt-os.windows.x86_32\dev_bundle\server-lib\node_modules\' -> 'C:\Users\trajano\AppData\Local\Temp\meteor-test-rund2fckq\.meteor\local\build\programs\server\node_modules'
    at Error (native)
    at Object.fs.symlinkSync (fs.js:897:18)
    at Object.wrapper (C:\tools\fs\files.js:1535:35)
    at Object.files.cp_r (C:\tools\fs\files.js:501:11)
    at C:\tools\fs\files.js:488:13
    at Array.forEach (native)
    at Object.files.cp_r (C:\tools\fs\files.js:475:25)
    at C:\tools\fs\files.js:488:13
    at Array.forEach (native)
    at Object.files.cp_r (C:\tools\fs\files.js:475:25)
    at C:\tools\fs\files.js:488:13
    at Array.forEach (native)
    at Object.files.cp_r (C:\tools\fs\files.js:475:25)
    at copyDirIntoTestRunnerApp (C:\tools\cli\commands.js:1692:15)
    at doTestCommand (C:\tools\cli\commands.js:1698:5)
    at Command.func (C:\tools\cli\commands.js:1533:10)
    at C:\tools\cli\main.js:1483:23

From a clone of the todos app. Going to try doing a meteor update to see if it will help. However, even after meteor update and meteor update --all-packages No luck as of yet.

From the stack trace it looks like it is a fundamental problem of Meteor itself rather than the driver since all the errors are from the C:\tools or native

[kfern]

Hi. Could you try open console as admin?

[trajano]

@kfern thanks that got it working. I'll continue when I can later. Is this new I don't recall ever having to do this before (running as admin I mean)

[abernix]

We really do not want anyone having to run meteor as Administrator as it's almost always caused problems somewhere along the line (on both Windows and Unix).

I think it's worth considering a recommendation to use dispatch:mocha here. If you have the opportunity to test, please do – the instructions on their GitHub seem to work for me. It's recently received more updates than the practicalmeteor implementation and breaks free of being locked into PhantomJS, which has become more and more problematic these days, it seems.

[abernix]

Thank you very much for checking into this, @trajano.

Marking as "Needs changes" based on the previous "don't merge" exclamation. This is just to avoid accidental merging – happy to dismiss this review as deemed appropriate. 😉

[trajano]

@abernix I'll look into this dispatch:mocha you speak of then maybe I'll just change it around to use that one instead.

[abernix]

Any updates on this, @trajano?

[trajano]

Closing this off, I don't have time to work on this anymore sorry.