Skip to content
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.

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

refactor: don't expose tempdir in common.state #903

Merged
merged 1 commit into from Nov 9, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/common.js
Expand Up @@ -42,11 +42,11 @@ var config = {
config.reset();
exports.config = config;

// Note: commands should generally consider these as read-only values.
var state = {
error: null,
errorCode: 0,
currentCmd: 'shell.js',
tempDir: null,
};
exports.state = state;

Expand Down
2 changes: 1 addition & 1 deletion src/exec.js
@@ -1,5 +1,5 @@
var common = require('./common');
var _tempDir = require('./tempdir');
var _tempDir = require('./tempdir').tempDir;
var _pwd = require('./pwd');
var path = require('path');
var fs = require('fs');
Expand Down
26 changes: 21 additions & 5 deletions src/tempdir.js
Expand Up @@ -24,6 +24,8 @@ function writeableDir(dir) {
}
}

// Variable to cache the tempdir value for successive lookups.
var cachedTempDir;

//@
//@ ### tempdir()
Expand All @@ -37,10 +39,9 @@ function writeableDir(dir) {
//@ Searches and returns string containing a writeable, platform-dependent temporary directory.
//@ Follows Python's [tempfile algorithm](http://docs.python.org/library/tempfile.html#tempfile.tempdir).
function _tempDir() {
var state = common.state;
if (state.tempDir) return state.tempDir; // from cache
if (cachedTempDir) return cachedTempDir;

state.tempDir = writeableDir(os.tmpdir()) ||
cachedTempDir = writeableDir(os.tmpdir()) ||
writeableDir(process.env.TMPDIR) ||
writeableDir(process.env.TEMP) ||
writeableDir(process.env.TMP) ||
Expand All @@ -54,6 +55,21 @@ function _tempDir() {
writeableDir('/usr/tmp') ||
writeableDir('.'); // last resort

return state.tempDir;
return cachedTempDir;
}
module.exports = _tempDir;

// Indicates if the tempdir value is currently cached. This is exposed for tests
// only. The return value should only be tested for truthiness.
function isCached() {
return cachedTempDir;
}

// Clears the cached tempDir value, if one is cached. This is exposed for tests
// only.
function clearCache() {
cachedTempDir = undefined;
}

module.exports.tempDir = _tempDir;
module.exports.isCached = isCached;
module.exports.clearCache = clearCache;
10 changes: 10 additions & 0 deletions test/tempdir.js
Expand Up @@ -3,6 +3,7 @@ import fs from 'fs';
import test from 'ava';

import shell from '..';
import { isCached, clearCache } from '../src/tempdir';

shell.config.silent = true;

Expand All @@ -19,3 +20,12 @@ test('basic usage', t => {
// It's a directory
t.truthy(shell.test('-d', tmp));
});

test('cache', t => {
clearCache(); // In case this runs after any test which relies on tempdir().
t.falsy(isCached());
const tmp1 = shell.tempdir();
t.truthy(isCached());
const tmp2 = shell.tempdir();
t.is(tmp1, tmp2);
});