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

patch grunt-mocha to fix 'test.titlePath' errors on test failure #2775

Merged
merged 2 commits into from
Apr 26, 2018

Conversation

Spongman
Copy link
Contributor

re: #2648, #2746, disqus/grunt-mocha#6, disqus/grunt-mocha#7,

this PR patches the grunt-mocha package with fixes so that test failures will now display the actual failure information instead of some internal test.titlePath is not a function error.

@limzykenneth
Copy link
Member

Seems like a good solution until we can maybe migrate to something that is either better maintained or something that doesn't rely on plugins so much.

@Spongman
Copy link
Contributor Author

Spongman commented Apr 13, 2018

indeed. somewhere i have a branch that uses headless-chrome for the test runner, but i had a bunch of issues with it not detecting properly when the process had ended.

@meiamsome
Copy link
Member

Alternate option: use the git url in the package.json (Preferably pinned to the particular commit) until this is published

@Spongman
Copy link
Contributor Author

That is possible. I'd have to create a fork and another branch since there's also the growl fix that isn't part of that grunt-mocha PR.

@meiamsome
Copy link
Member

@Spongman can you explain what the growl fix achieves? Surely the previous code goes through the catch section and sets it to an empty function anyway?

@Spongman
Copy link
Contributor Author

Spongman commented Apr 15, 2018

Actually the require succeeds, but then growl just crashes when the test fails, so I just removed it.

@meiamsome
Copy link
Member

meiamsome commented Apr 15, 2018

Hmm, that's a bit odd considering growl isn't in the deps. Do you have it on global install? Could be a version mismatch if so.
I'd never even consider that there'd be such issues when not including an optional dependency...

@Spongman
Copy link
Contributor Author

Spongman commented Apr 15, 2018

i get this:

> yarn why growl
yarn why v1.5.1
[1/4] Why do we have the module "growl"...?
[2/4] Initialising dependency graph...
[3/4] Finding dependency...
[4/4] Calculating file sizes...
=> Found "growl@1.10.3"
info Reasons this module exists
   - "grunt-mocha#mocha" depends on it
   - Hoisted from "grunt-mocha#mocha#growl"

and this when a test fails:

>> 1/561 tests failed (134.68s)
An error occured. { Error: spawn growlnotify ENOENT
    at Process.ChildProcess._handle.onexit (internal/child_process.js:201:19)
    at onErrorNT (internal/child_process.js:379:16)
    at process._tickCallback (internal/process/next_tick.js:178:19)
  errno: 'ENOENT',
  code: 'ENOENT',
  syscall: 'spawn growlnotify',
  path: 'growlnotify',
  spawnargs:
   [ '/i:E:\\play\\p5.js\\node_modules\\grunt-mocha\\growl\\error.png',
     '1/561 tests failed (134.68s)',
     '/t:Failure in yui' ] }
An error occured. { Error: spawn growlnotify ENOENT
    at Process.ChildProcess._handle.onexit (internal/child_process.js:201:19)
    at onErrorNT (internal/child_process.js:379:16)
    at process._tickCallback (internal/process/next_tick.js:178:19)
  errno: 'ENOENT',
  code: 'ENOENT',
  syscall: 'spawn growlnotify',
  path: 'growlnotify',
  spawnargs:
   [ '/i:E:\\play\\p5.js\\node_modules\\grunt-mocha\\growl\\error.png',
     '1/561 tests failed (134.68s)',
     '/t:1/561 tests failed (134.68s)' ] }
Warning: Task "mocha:yui" failed. Use --force to continue.

Aborted due to warnings.
error An unexpected error occurred: "Command failed.
Exit code: 3
Command: C:\\WINDOWS\\system32\\cmd.exe
Arguments: /d /s /c grunt --quiet
Directory: E:\\play\\p5.js
Output:
".
info If you think this is a bug, please open a bug report with the information provided in "E:\\play\\p5.js\\yarn-error.log".
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

growl is not installed globally.

@Spongman
Copy link
Contributor Author

Spongman commented Apr 24, 2018

any chance we can get this merged so we can actually see test errors in travis?

@lmccart
Copy link
Member

lmccart commented Apr 25, 2018

thank you. as a reminder, issues need to be opened before pull requests are opened and tagged with the issue. this is necessary for tracking development and keeping discussion clear. thanks!

@lmccart lmccart closed this Apr 25, 2018
@Spongman
Copy link
Contributor Author

am I missing something here? I listed two issues above that this fix addresses.

@Spongman
Copy link
Contributor Author

Here's another: #2812

@lmccart
Copy link
Member

lmccart commented Apr 25, 2018

please open an issue that documents the specific problem you're solving with this pr. if it's solving a related point that comes up in a completely separate issue thread it is difficult to track. thanks.

@Spongman
Copy link
Contributor Author

closes #2820

@lmccart lmccart reopened this Apr 26, 2018
@lmccart lmccart merged commit fb83289 into processing:master Apr 26, 2018
@Spongman Spongman deleted the fix-mocha-growl branch May 11, 2018 06:32
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.

None yet

4 participants