Skip to content

Make karma tests work on windows #2

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Make karma tests work on windows #2

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion civicrm-cv.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@ var execPromise = require('child-process-promise').exec;
var execSync = require('child_process').execSync;

var escape = function(cmd) {
return '\'' + cmd.replace(/'/g, "'\\''") + '\'';
if (process.platform === "win32") {
return '"' + cmd.replace(/"/g, "'") + '"';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, a Windows patch. Cool. 👍

With this regex, though... suppose the content includes a double-quote. Won't this convert it to a single-quote -- and convey different substance? I feel like (on at least a few occasions) I've used cv() to call php:eval, and that that requires some of the params to be passed through quite precisely.

I was curious about how one is supposed to escape a string on Windows, and found https://stackoverflow.com/a/31413730/4195300 -- 😱 Maybe we should either:

  • Figure out which notation is specifically appropriate for launching PHP subcommands on Windows
  • Rework the file so that we don't need to call serializeArgs() or escape() -- basically replacing execSync() with execFileSyc(); the latter should bypass escaping issues.

} else {
return '\'' + cmd.replace(/'/g, "'\\''") + '\'';
}
};

function serializeArgs(args) {
Expand Down