Gpii 12 #59
Gpii 12 #59
Conversation
… comments on function.
|
DDDD! This is an absolutely brilliant PUL request. All it needs to get in is some kind of test case! What I suggest is - from here http://www.chiark.greenend.org.uk/~sgtatham/putty/download.html - download puttygen.exe (180K), rename it to something unusual, and issue a test case that attempts to kill it, and returns success if it (shortly) disappears from the process list (as reported, for example, by our standard tasklist /fi "STATUS eq RUNNING" /FI "IMAGENAME eq THING command line that is currently cut n pasted into all our acceptance tests.... |
|
AAAA! I was wondering a bit about that. What do you think about a test case for that function just launching and killing an app in windows that is always there such as notepad.exe or maybe a calculator if there is one? Or do you think there is too much worry about closing an app someone has open while developing? (It seems like there must be some innocuous app that always ships with windows we can use to test?) Also, if I"m correct there are no unit tests yet for WindowsUtilities.js, so I need to add a tests directory for that particular module. |
|
Well, I don't think that the behaviour of killing an app that the user may already have running is acceptable for a test case : P |
…ed BSD program putty, also adding shelljs to the list of development dependencies.
|
@amb26 I haven't the tests you recommended. Please take another look at this review when you have a chance. Thanks! |
|
Great work Hens, thanks for this. |
|
@amb26 As requested I've put a comment on that async exec call. |
…yType from node-ffi since it isn't an actual prototypical type that requires instantiation with new.
|
@kaspermarkus Those jshint fixes should be fixed and part of this pull now. Thanks! |
| return false; | ||
| } | ||
| }; | ||
| jqUnit.assertDeepEq( "We've haven't started it yet.", false, checkForTask()); |
There was a problem hiding this comment.
This pull should never have been merged. It turns out that all of these assertions are faulty - assertDeepEq will always pass when given a primitive value. This is a QUnit bug. These assertions pass whether they are true or false.
There was a problem hiding this comment.
http://issues.gpii.net/browse/GPII-12