From a8a56065157e24e36f0c6f5ce1108d6cf9eb0247 Mon Sep 17 00:00:00 2001 From: Steve Gravrock Date: Fri, 28 Oct 2022 18:10:13 -0700 Subject: [PATCH] Warn when backslashes are used in paths/globs on Windows 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. --- bin/jasmine.js | 17 ++++++- lib/command.js | 23 ++++++--- lib/jasmine.js | 25 ++++++++- spec/command_spec.js | 49 +++++++++++++++++- spec/jasmine_spec.js | 118 +++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 222 insertions(+), 10 deletions(-) diff --git a/bin/jasmine.js b/bin/jasmine.js index a0ecd61a..b1edc274 100755 --- a/bin/jasmine.js +++ b/bin/jasmine.js @@ -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)); diff --git a/lib/command.js b/lib/command.js index ac273afd..c50ed0b9 100644 --- a/lib/command.js +++ b/lib/command.js @@ -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; @@ -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(', ')); @@ -63,7 +66,7 @@ function isFileArg(arg) { return arg.indexOf('--') !== 0 && !isEnvironmentVariable(arg); } -function parseOptions(argv) { +function parseOptions(argv, isWindows) { let files = [], helpers = [], requires = [], @@ -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); } @@ -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, '/'); +} diff --git a/lib/jasmine.js b/lib/jasmine.js index 3a90ad5a..e124464d 100644 --- a/lib/jasmine.js +++ b/lib/jasmine.js @@ -1,3 +1,4 @@ +const os = require('os'); const path = require('path'); const util = require('util'); const glob = require('glob'); @@ -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) { @@ -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 = []; @@ -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. @@ -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}.`); + } + } } /** @@ -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]; diff --git a/spec/command_spec.js b/spec/command_spec.js index 802a2371..6ddbfb3d 100644 --- a/spec/command_spec.js +++ b/spec/command_spec.js @@ -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']); @@ -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) diff --git a/spec/jasmine_spec.js b/spec/jasmine_spec.js index 49440e3b..5c95b87c 100644 --- a/spec/jasmine_spec.js +++ b/spec/jasmine_spec.js @@ -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(); + }); });