Fixes CLI tests for windows #560
Conversation
|
Thanks @kookookchoozeus! I think it would be cleaner to hijack var child_process = require('child_process')
function exec () {
arguments[0] = changeOS(arguments[0])
return child_process.exec.apply(child_process, arguments)
} |
| if(process.platform === 'win32') { | ||
| return command. | ||
| replace(/bin\/mustache/g, 'node bin\\mustache'). | ||
| replace(/cat /g, 'type '). |
There was a problem hiding this comment.
I think it would be preferable to wrap the regexes with \b symbols, like /\bcat\b/. Right now any word/filename ending with cat will instead end with type. It's an unlikely scenario, but it's existence makes the solution not as clean, I feel.
…nd made replacing 'cat' safer with word boundaries
|
Thanks @dasilvacontin, very good suggestions that I should have realized myself, haha. |
| assert.notEqual(stderr.indexOf('Could not find file: test/_files/non-existing-template.mustache'), -1); | ||
| console.log(stderr); | ||
| assert.notEqual(stderr.indexOf('Could not find file:'), -1); | ||
| assert.notEqual(stderr.indexOf(changeForOS('test/_files/non-existing-template.mustache')), -1); |
There was a problem hiding this comment.
Could you explain why the assertion was split into two different ones?
There was a problem hiding this comment.
The filepath is absolute on Windows (e.g. C:\Users\you\path\to\mustache\test_files...)
There was a problem hiding this comment.
I see. That's not very obvious, just by reading the code..
I was going to suggest using a regex, eg /Could not find file.*non-existing-template\.mustache/., but don't worry about it, I'm going to refactor these assertions after we merge this PR (using unexpected or some nice assertion library).
There was a problem hiding this comment.
My initial thought was to use a regex as well, but then you'd also have to take the backslash problem into account with [\/\\]{1,2}
There was a problem hiding this comment.
Not if you only add the last part of the file's path to the regex, you have no worries about the slashes that way. :)
|
And I just noticed I used |
|
Update: made the changes suggested above, hoping to get this merged soon so I can update #558 |
|
LGTM, thanks @kookookchoozeus! :) |
Fixes #559
All that was needed was a helper function for transforming *nix commands into windows commands:
bin/mustacheis run withnodecatwithtypeLeaves commands as they are for *nix, so won't break the tests there
Also replaced the
Could not find filestrings to be compatible for windows