Skip to content

Commit

Permalink
Warn when backslashes are used in paths/globs on Windows
Browse files Browse the repository at this point in the history
Backslashes behave inconsistently between OSes in the version of glob
that Jasmine currently uses, and the next major verison of glob will
treat them as escape sequences rather than path separators on all OSes.
  • Loading branch information
sgravrock committed Oct 29, 2022
1 parent da0db7f commit a8a5606
Show file tree
Hide file tree
Showing 5 changed files with 222 additions and 10 deletions.
17 changes: 15 additions & 2 deletions bin/jasmine.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,24 @@
#!/usr/bin/env node

const path = require('path');
const os = require('os');
const Command = require('../lib/command');
const Jasmine = require('../lib/jasmine');

const jasmine = new Jasmine({ projectBaseDir: path.resolve() });
let projectBaseDir = path.resolve();

if (os.platform() === 'win32') {
// Future versions of glob will interpret backslashes as escape sequences on
// all platforms, and Jasmine warns about them. Convert to slashes to avoid
// the warning and future behavior change.
projectBaseDir = projectBaseDir.replace(/\\/g, '/');
}

const jasmine = new Jasmine({ projectBaseDir });
const examplesDir = path.join(path.dirname(require.resolve('jasmine-core')), 'jasmine-core', 'example', 'node_example');
const command = new Command(path.resolve(), examplesDir, console.log);
const command = new Command(path.resolve(), examplesDir, {
print: console.log,
platform: os.platform,
});

command.run(jasmine, process.argv.slice(2));
23 changes: 17 additions & 6 deletions lib/command.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,12 @@ const subCommands = {
}
};

function Command(projectBaseDir, examplesDir, print) {
this.projectBaseDir = projectBaseDir;
this.specDir = path.join(projectBaseDir, 'spec');
function Command(projectBaseDir, examplesDir, deps) {
const {print, platform} = deps;
const isWindows = platform() === 'win32';

this.projectBaseDir = isWindows ? unWindows(projectBaseDir) : projectBaseDir;
this.specDir = `${this.projectBaseDir}/spec`;

const command = this;

Expand All @@ -46,7 +49,7 @@ function Command(projectBaseDir, examplesDir, print) {
if (commandToRun) {
commandToRun.action({jasmine: jasmine, projectBaseDir: command.projectBaseDir, specDir: command.specDir, examplesDir: examplesDir, print: print});
} else {
const env = parseOptions(commands);
const env = parseOptions(commands, isWindows);
if (env.unknownOptions.length > 0) {
process.exitCode = 1;
print('Unknown options: ' + env.unknownOptions.join(', '));
Expand All @@ -63,7 +66,7 @@ function isFileArg(arg) {
return arg.indexOf('--') !== 0 && !isEnvironmentVariable(arg);
}

function parseOptions(argv) {
function parseOptions(argv, isWindows) {
let files = [],
helpers = [],
requires = [],
Expand Down Expand Up @@ -100,7 +103,7 @@ function parseOptions(argv) {
} else if (arg === '--') {
break;
} else if (isFileArg(arg)) {
files.push(arg);
files.push(isWindows ? unWindows(arg) : arg);
} else if (!isEnvironmentVariable(arg)) {
unknownOptions.push(arg);
}
Expand Down Expand Up @@ -315,3 +318,11 @@ function setEnvironmentVariables(commands) {
}
});
}

// Future versions of glob will interpret backslashes as escape sequences on
// all platforms, and Jasmine warns about them. Convert to slashes to avoid
// the warning and future behavior change. Should only be called when running
// on Windows.
function unWindows(projectBaseDir) {
return projectBaseDir.replace(/\\/g, '/');
}
25 changes: 24 additions & 1 deletion lib/jasmine.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
const os = require('os');
const path = require('path');
const util = require('util');
const glob = require('glob');
Expand Down Expand Up @@ -42,6 +43,7 @@ class Jasmine {
constructor(options) {
options = options || {};
this.loader = options.loader || new Loader();
this.isWindows_ = (options.platform || os.platform)() === 'win32';
const jasmineCore = options.jasmineCore || require('jasmine-core');

if (options.globals === false) {
Expand All @@ -50,7 +52,13 @@ class Jasmine {
this.jasmine = jasmineCore.boot(jasmineCore);
}

this.projectBaseDir = options.projectBaseDir || path.resolve();
if (options.projectBaseDir) {
this.validatePath_(options.projectBaseDir);
this.projectBaseDir = options.projectBaseDir;
} else {
this.projectBaseDir = (options.getcwd || path.resolve)();
}

this.specDir = '';
this.specFiles = [];
this.helperFiles = [];
Expand Down Expand Up @@ -275,6 +283,7 @@ class Jasmine {
* @type string | undefined
*/
this.specDir = config.spec_dir || this.specDir;
this.validatePath_(this.specDir);

/**
* Whether to fail specs that contain no expectations.
Expand Down Expand Up @@ -497,6 +506,16 @@ class Jasmine {

return overallResult;
}

validatePath_(path) {
if (this.isWindows_ && path.includes('\\')) {
const fixed = path.replace(/\\/g, '/');
console.warn('Backslashes in ' +
'file paths behave inconsistently between platforms and might not be ' +
'treated as directory separators in a future version. Consider ' +
`changing ${path} to ${fixed}.`);
}
}
}

/**
Expand All @@ -520,6 +539,10 @@ Jasmine.prototype.addMatchingHelperFiles = addFiles('helperFiles');

function addFiles(kind) {
return function (files) {
for (const f of files) {
this.validatePath_(f);
}

const jasmineRunner = this;
const fileArr = this[kind];

Expand Down
49 changes: 48 additions & 1 deletion spec/command_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,10 @@ describe('command', function() {
};
}());

this.command = new Command(projectBaseDir, examplesDir, this.out.print);
this.command = new Command(projectBaseDir, examplesDir, {
print: this.out.print,
platform: () => 'Oberon'
});

this.fakeJasmine = jasmine.createSpyObj('jasmine', ['loadConfigFile', 'addMatchingHelperFiles', 'addRequires', 'showColors', 'execute',
'randomizeTests', 'seed', 'coreVersion', 'clearReporters', 'addReporter']);
Expand Down Expand Up @@ -385,6 +388,50 @@ describe('command', function() {
expect(this.fakeJasmine.seed).toHaveBeenCalledWith('12345');
});
});

describe('Path handling', function() {
describe('On Windows', function () {
beforeEach(function() {
this.deps = {
print: this.out.print,
platform: () => 'win32'
};
});

it('replaces backslashes in the project base dir with slashes', function() {
const subject = new Command('foo\\bar', '', this.deps);
expect(subject.projectBaseDir).toEqual('foo/bar');
expect(subject.specDir).toEqual('foo/bar/spec');
});

it('replaces backslashes in spec file paths from the command line', async function() {
const subject = new Command('arbitrary', '', this.deps);
await subject.run(this.fakeJasmine, ['somedir\\somespec.js']);
expect(this.fakeJasmine.execute).toHaveBeenCalledWith(['somedir/somespec.js'], undefined);
});
});

describe('On non-Windows systems', function () {
beforeEach(function() {
this.deps = {
print: this.out.print,
platform: () => 'BeOS'
};
});

it('does not replace backslashes in the project base dir', function() {
const subject = new Command('foo\\bar', '', this.deps);
expect(subject.projectBaseDir).toEqual('foo\\bar');
expect(subject.specDir).toEqual('foo\\bar/spec');
});

it('does not replace backslashes in spec file paths from the command line', async function() {
const subject = new Command('arbitrary', '', this.deps);
await subject.run(this.fakeJasmine, ['somedir\\somespec.js']);
expect(this.fakeJasmine.execute).toHaveBeenCalledWith(['somedir\\somespec.js'], undefined);
});
});
});
});

// Adapted from Sindre Sorhus's escape-string-regexp (MIT license)
Expand Down
118 changes: 118 additions & 0 deletions spec/jasmine_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -630,4 +630,122 @@ describe('Jasmine', function() {
});
});
});

describe('When running on Windows', function() {
beforeEach(function() {
spyOn(console, 'warn');
});

function windows() {
return 'win32';
}

it('warns about backslashes in the configured project base dir', function() {
new Jasmine({
projectBaseDir: 'c:\\foo\\bar',
platform: windows,
jasmineCore: this.fakeJasmineCore,
});
expect(console.warn).toHaveBeenCalledWith('Backslashes in ' +
'file paths behave inconsistently between platforms and might not be ' +
'treated as directory separators in a future version. Consider ' +
'changing c:\\foo\\bar to c:/foo/bar.');
});

it('does not warn about backslashes in the current working directory', function() {
const jasmine = new Jasmine({
getcwd: () => 'c:\\foo\\bar',
platform: windows,
jasmineCore: this.fakeJasmineCore,
});
expect(jasmine.projectBaseDir).toEqual('c:\\foo\\bar');
expect(console.warn).not.toHaveBeenCalled();
});

it('warns about backslashes in spec_dir', function() {
const jasmine = new Jasmine({
platform: windows,
jasmineCore: this.fakeJasmineCore,
});
jasmine.loadConfig({
spec_dir: 'foo\\bar',
});

expect(console.warn).toHaveBeenCalledWith('Backslashes in ' +
'file paths behave inconsistently between platforms and might not be ' +
'treated as directory separators in a future version. Consider ' +
'changing foo\\bar to foo/bar.');
});

it('warns about backslashes in helpers', function() {
const jasmine = new Jasmine({
platform: windows,
jasmineCore: this.fakeJasmineCore,
});

jasmine.loadConfig({
helpers: ['foo\\bar']
});
expect(console.warn).toHaveBeenCalledWith('Backslashes in ' +
'file paths behave inconsistently between platforms and might not be ' +
'treated as directory separators in a future version. Consider ' +
'changing foo\\bar to foo/bar.');

jasmine.addMatchingHelperFiles(['foo\\baz']);
expect(console.warn).toHaveBeenCalledWith('Backslashes in ' +
'file paths behave inconsistently between platforms and might not be ' +
'treated as directory separators in a future version. Consider ' +
'changing foo\\baz to foo/baz.');
});

it('warns about backslashes in spec_files', function() {
const jasmine = new Jasmine({
platform: windows,
jasmineCore: this.fakeJasmineCore,
});

jasmine.loadConfig({
spec_files: ['foo\\bar']
});
expect(console.warn).toHaveBeenCalledWith('Backslashes in ' +
'file paths behave inconsistently between platforms and might not be ' +
'treated as directory separators in a future version. Consider ' +
'changing foo\\bar to foo/bar.');

jasmine.addMatchingSpecFiles(['foo\\baz']);
expect(console.warn).toHaveBeenCalledWith('Backslashes in ' +
'file paths behave inconsistently between platforms and might not be ' +
'treated as directory separators in a future version. Consider ' +
'changing foo\\baz to foo/baz.');
});

it('does not warn if no configured path contains backslashes', function() {
const jasmine = new Jasmine({
projectBaseDir: 'c:/foo/bar',
platform: windows,
jasmineCore: this.fakeJasmineCore,
});
jasmine.loadConfig({
spec_dir: 'foo/bar',
spec_files: ['baz/qux'],
helpers: ['waldo/fred']
});
expect(console.warn).not.toHaveBeenCalled();
});
});

it('does not warn about backslashes when not running on Windows', function() {
spyOn(console, 'warn');
const jasmine = new Jasmine({
projectBaseDir: 'foo\\bar',
platform: () => 'NetWare',
jasmineCore: this.fakeJasmineCore,
});
jasmine.loadConfig({
spec_dir: 'foo\\bar',
spec_files: ['baz\\qux'],
helpers: ['waldo\\fred']
});
expect(console.warn).not.toHaveBeenCalled();
});
});

0 comments on commit a8a5606

Please sign in to comment.