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

Update dev dependencies (grunt, grunt-complexity, grunt-mocha) #437

Merged
merged 1 commit into from
Nov 26, 2018
Merged

Update dev dependencies (grunt, grunt-complexity, grunt-mocha) #437

merged 1 commit into from
Nov 26, 2018

Conversation

kleinfreund
Copy link
Contributor

@kleinfreund kleinfreund commented Nov 26, 2018

This pull request:

  • Updates all development dependencies.
  • Updates the test HTML file to refer to mocha.js/mocha.css from the local package installation.
  • Replaces grunt mocha with npm test (which executes grunt mocha via npm script in the package.json) in the readme.

This fixes the tests logging a Fatal error: Callback must be a function without a tracable source. Also, tests are currently reported as failing when actually all tests pass. This is fixed as well.

Full output with the current state of the repository:

phil@RAUMSTATION in ~/dev/repos/mousetrap on master!
👻 npm test

> [email protected] test /home/phil/dev/repos/mousetrap
> grunt mocha

Running "mocha:mousetrap" (mocha) task
Testing: tests/mousetrap.html
 60  -_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_,------,
 0   -_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_|   /\_/\ 
 0   -_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-^|__( ^ .^) 
     -_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-  ""  "" 

  60 tests complete (151 ms)

Fatal error: Callback must be a function
npm ERR! Test failed.  See above for more details.

@ccampbell
Copy link
Owner

Thanks for this! I am happy to merge. I just tested it locally though, and I am seeing this error after the tests run:

An error occured. { [Error: spawn growlnotify ENOENT
  at Process.ChildProcess._handle.onexit (internal/child_process.js:230:19)
  at onErrorNT (internal/child_process.js:407:16)
  at process._tickCallback (internal/process/next_tick.js:63:19)
]
  errno: 'ENOENT',
  code: 'ENOENT',
  syscall: 'spawn growlnotify',
  path: 'growlnotify',
  spawnargs:
   [ '--image',
     '/Users/craig/Sites/mousetrap/node_modules/grunt-mocha/growl/ok.png',
     '-m',
     '60 passed! (0.27s)',
     '60 passed! (0.27s)' ] }

Is there some dependency on growl that was added?

@kleinfreund
Copy link
Contributor Author

kleinfreund commented Nov 26, 2018

Oh! I wouldn’t know about that. There is no such error when I run the tests. Which Node version are you running?

Some people in mochajs/mocha#3088 suggest setting growlOnSuccess to false. But I don’t know whether this is related. I may have time to look into this tomorrow.

mocha: {
    all: {
        src: ['tests/testrunner.html']
    },
    options: {
        run: true,
        growlOnSuccess: false
    }
}

@ccampbell ccampbell merged commit 2baef15 into ccampbell:master Nov 26, 2018
ccampbell added a commit that referenced this pull request Nov 26, 2018
@ccampbell
Copy link
Owner

That did it. Thank you!

@kleinfreund kleinfreund deleted the update-dev-dependencies branch November 26, 2018 23:08
@kleinfreund
Copy link
Contributor Author

Hey @ccampbell. Unfortunately, this pull request broke the test HTML on https://rawgit.com/ccampbell/mousetrap/master/tests/mousetrap.html because I’m referencing the mocha.js/mocha.css files from a node_modules directory that doesn’t exist in the repository without running npm install.

Should I copy the files from the node_modules directory and reference them like the file did before?

@ccampbell
Copy link
Owner

Hmm. Or we could just remove the link from the README. I doubt that many people actually use it. I also read that rawgit.com is shutting down soon so it will stop working anyway eventually.

@kleinfreund
Copy link
Contributor Author

@ccampbell Right, I read that, too. Then I don’t think there is a point in maintaining it.

alanhe421 added a commit to alanhe421/mousetrap that referenced this pull request Nov 13, 2023
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

Successfully merging this pull request may close these issues.

2 participants