fix(reporter): allow prefix overrides on win32 #64
Conversation
- no need to provide path into node_modules
- remove single-quotes around glob expressions
- tests were not being run with them in place
…and istanbul is failing to find _mocha)
- I'm seeing it automatically brought in on win32, but it's
missing on Linux
|
Apologies for the last few trailing commits: it appears as if mocha-runner, with a current Node (7.x) on win32, pulls in mocha; but not with a lower version of Node (4.8.0) on Linux, causing Travis to fail on the coverage script. |
|
This PR has been open nearly a month now and I still can't read output on a client's TFS server. I was going to fork on npmjs.com, but I've found a very similar project, I'm a little dismayed that no attention was given to this PR. Not only did I fix a long-standing issue (though I'm not the first to provide a patch for the behavior), I added coverage on other areas of code which weren't covered in the past because I saw historical discussions about the desire of the project to maintain decent coverage. It feels a little like the effort was wasted, to be honest. |
|
@fluffynuts It looks like your PR is good to go. Sorry, I've not really paid attention to this repo as I don't use the reporter day to day anymore. Please don't be discouraged. If you want to help increase coverage I would suggest a separate PR but I'm okay with merging this in if I pull it down and everything works as expected. Also, it's good to remember that this repo is actually used by a lot of people (~444k downloads on npm this month). So introducing changes has to be tested before things are pushed and published as a new version. |
|
As a note, there are still some tests failing on the Windows 10 machine (this is what I'm developing on locally). I had this problem with a prior PR I merged and a case needs to be explicitly declared for the tests failing only on Windows 10. Here is the output: |
|
This is strange -- I spent a reasonable amount of time ensuring that the tests worked across windows and linux (hence the changes to the script block in package.json). Also, re-running the tests on my win10 laptop (both from my fork and from a fresh clone of this repository), everything passes. I'm currently downloading 7.8.0 (latest stable) and will then try the last stable (6.10.1) |
|
(btw, thanks for accepting the PR -- if there is an issue caused by something like expecting too high a node version, I'll sort it out) |
|
It is strange. I had no idea what was happening, but it seemed to take place after the upgrade of the colors dependency PR #59 . I didn't really have time to look into it, but can make an issue for it if need be. I think you might be on to something, as I am running node 6.10.0. I don't have nvm installed on my machine yet, but can definitely find a way if you need help! Thanks for your patience 😄 If it makes you feel better I just merged a few PRs a month ago that were sitting for half a year lol I'll make sure to pay more attention if someone has put in the work for a PR, my apologies again for the delay! |
|
Googling around a bit, it looks like this is taken care of by libuv, which node is using to deal with ansi color escape sequences on Windows, where the console has traditionally not dealt with those sequences. I happen to know that traditional win32 color console output is tedious, having done it enough from c and c#. |
Similar to #57, which seems to have been abandoned, though implemented quite differently.
In addition to adding tests for the functionality I wanted to add, I've also: