Run 'fail' test on Windows too #17
Conversation
alextes
force-pushed
the
always-run-fail-test
branch
2 times, most recently
from
e691843
to
a96e910
Compare
sindresorhus
changed the title
|
Can you rebase from master so you'll get the test delay. |
|
I will. Was waiting to see if you would 馃槈. (I'll mind my capitalization 馃槄.) |
alextes
force-pushed
the
always-run-fail-test
branch
from
a96e910
to
3196a95
Compare
I think it's an additional protection to not kill ourselves when killing by name. The JS filter only filters PIDs. |
|
Alright, can I add |
|
@alextes No, process title by default is just generic |
|
Seems like the tests here are failing on Windows. |
|
They pass locally on win10. I'm confused. Leave this one for now and let me do some digging. |
Ah. When killing 'node' we should kill all node processes, except our own, which can only be done by excluding our pid. We did this on Windows, but not for the rest. I had a quick look. Don't think this can be done for Since leaving the filter in would mean giving up on getting a 'not found' error, and leaving in inconsistent behavior, how about I open a new issue (last one I promise 馃槗), with the suggestion to kill PID's excluding our own in case of the input being 'node'? |
|
Sure |
alextes
force-pushed
the
always-run-fail-test
branch
4 times, most recently
from
45053fa
to
648bbac
Compare
Adding a filter that correctly, doesn't match our process id, makes taskkill exit with "INFO: No tasks running with the specified criteria." even if no processes were killed. We'll find another way to prevent killing ourselves if killing 'node'.
Mocks the global process.pid to make sure a failing test wouldn't kill the test itself.
alextes
force-pushed
the
always-run-fail-test
branch
from
648bbac
to
68e48dd
Compare
alextes
force-pushed
the
always-run-fail-test
branch
from
68e48dd
to
ccbc6e3
Compare
|
@sindresorhus this is ready to merge now. Most times AppVeyor will not kill 'notepad.exe' without force. I do not understand it. |
|
Awesome :)
猬嗭笍 Make sure to open that issue ;) |
Moves the 'fail' test from non-windows only to running on all systems.
taskkillalready does throw when not finding a process to kill. However, we were adding a filter not to kill ourselves. The filter for some reason feels that it doesn't have to kill anything to be successful 馃檨.We currently filter on our pid twice. Maybe I'm missing something obvious, after all, I also missed the fact we're filtering out our pid twice. It looks to me like there is no reason.
taskkillfilters.fixes #16