Bug#847749: ITP: node-user-home -- Get the path to the user home directory
On 12/11/2016 06:29 PM, Sruthi Chandran wrote:
> On 12/11/2016 06:18 PM, Guus Sliepen wrote:
>> Isn't s/user-home/os-homedir/ not enough? In any case, maybe you should
>> try to get upstream to switch to os-homedir instead.
> os-homedir is not packaged, we have been patching that with os.homedir.
> In this case, the patching is partially working, 4 tests are passing and
> 2 are failing.
>
> I have pushed v8flags to alioth [1]. Please have a look. Test is "mocha
> -R spec test.js".
Side-note 1: there appears to be a dependency problem here when running
tests: for mocha to work (at least in this case), you also need the
package chai to be installed (otherwise it will fail with an error).
I think this is because test.js requires chai, but I'm not sure.
(I suspect the dependencies of mocha and node-v8flags themselves are
OK because it's not required for the package directly. OTOH you could
add the test suite to the package's autopkgtests and then add chai and
mocha to the Depends there.)
Problem 1:
- cleanup hook is missing some patching out:
delete require.cache[require.resolve('user-home')];
If that line is not removed, then the cleanup hook will fail with
require.resolve('user-home'), because that module is not installed.
Problem 2:
- after patching that out I get the following test results:
v8flags
✓ should cache and call back with the v8 flags for the running process
✓ should not append the file when multiple calls happen concurrently and the config file does not yet exist
1) should fall back to writing to a temp dir if user home can't be found
✓ should fall back to writing to a temp dir if user home is unwriteable
2) should return flags even if an error is thrown
✓ should back with an empty array if the runtime is electron
The following errors occur:
1) v8flags should fall back to writing to a temp dir if user home can't be found:
Uncaught AssertionError: expected false to be true
at /srv/external/Debian/Debugging/node-user-home-replacement/node-v8flags/test.js:77:46
at /srv/external/Debian/Debugging/node-user-home-replacement/node-v8flags/index.js:108:14
at /srv/external/Debian/Debugging/node-user-home-replacement/node-v8flags/index.js:36:12
at /srv/external/Debian/Debugging/node-user-home-replacement/node-v8flags/index.js:47:7
at nextTickCallbackWith0Args (node.js:420:9)
at process._tickCallback (node.js:349:13)
2) v8flags should return flags even if an error is thrown:
Uncaught AssertionError: expected null not to be null
at /srv/external/Debian/Debugging/node-user-home-replacement/node-v8flags/test.js:98:28
at /srv/external/Debian/Debugging/node-user-home-replacement/node-v8flags/index.js:108:14
at /srv/external/Debian/Debugging/node-user-home-replacement/node-v8flags/index.js:36:12
at /srv/external/Debian/Debugging/node-user-home-replacement/node-v8flags/index.js:47:7
at nextTickCallbackWith0Args (node.js:420:9)
at process._tickCallback (node.js:349:13)
In test.js there is a routine eraseHome() which unsets a lot of environment
variables. This is to trick the user-home module (which we rejected from
Debian, if you remember) to think it can't fetch the user's home directory,
and therefore trigger the fallback.
This is problematic because os.homedir() falls back to reading /etc/passwd
(as it should, that's a feature, not a bug), so it won't fail. That means
that the fallback to a temporary directory won't happen. This means that
in the first case the fallback won't be triggered, and the expectation of
the test is wrong.
The second failure is related: since it tries to trigger the "home dir
doesn't exist" error to see if the module will succeed anyway without
throwing an exception, it does expect the error in the callback passed
to the module to be non-null - since it expects an error to have occurred.
However, in this case there was no error.
This is not a failure in the module itself, this is only a problem with
the test suite.
There's an easy fix though: monkey-patch os.homedir to a function that
just returns null in eraseHome(), and restore it in cleanup().
I've attached an updated use-os-homedir.patch that does this (including
the removal of the require.resolve() line above), and the test suite
now passes for all 6 tests:
v8flags
✓ should cache and call back with the v8 flags for the running process
✓ should not append the file when multiple calls happen concurrently and the config file does not yet exist
✓ should fall back to writing to a temp dir if user home can't be found
✓ should fall back to writing to a temp dir if user home is unwriteable
✓ should return flags even if an error is thrown
✓ should back with an empty array if the runtime is electron
6 passing (61ms)
Hope that helps.
Regards,
Christian
use os.homedir instead of user-home
--- a/index.js
+++ b/index.js
@@ -26,7 +26,7 @@ function fail (err) {
}
function openConfig (cb) {
- var userHome = require('user-home');
+ var userHome = os.homedir();
if (!userHome) {
return tryOpenConfig(path.join(os.tmpdir(), configfile), cb);
}
--- a/test.js
+++ b/test.js
@@ -7,6 +7,8 @@ const expect = require('chai').expect;
const env = process.env;
+const old_os_homedir = os.homedir;
+
function eraseHome() {
delete env.HOME;
delete env.USERPROFILE;
@@ -16,6 +18,8 @@ function eraseHome() {
delete env.USER;
delete env.LNAME;
delete env.USERNAME;
+
+ os.homedir = function() { return null; }
}
function setTemp(dir) {
@@ -24,7 +28,7 @@ function setTemp(dir) {
function cleanup () {
var v8flags = require('./');
- var userHome = require('user-home');
+ var userHome = os.homedir();
if (userHome === null) userHome = __dirname;
var files = [
@@ -37,7 +41,7 @@ function cleanup () {
} catch (e) {}
});
- delete require.cache[require.resolve('user-home')];
+ os.homedir = old_os_homedir;
delete process.versions.electron;
}
@@ -47,7 +51,7 @@ describe('v8flags', function () {
it('should cache and call back with the v8 flags for the running process', function (done) {
var v8flags = require('./');
- var configfile = path.resolve(require('user-home'), v8flags.configfile);
+ var configfile = path.resolve(os.homedir(), v8flags.configfile);
v8flags(function (err, flags) {
expect(flags).to.be.a('array');
expect(fs.existsSync(configfile)).to.be.true;
@@ -62,7 +66,7 @@ describe('v8flags', function () {
it('should not append the file when multiple calls happen concurrently and the config file does not yet exist', function (done) {
var v8flags = require('./');
- var configfile = path.resolve(require('user-home'), v8flags.configfile);
+ var configfile = path.resolve(os.homedir(), v8flags.configfile);
async.parallel([v8flags, v8flags, v8flags], function (err, result) {
v8flags(function (err2, res) {
done();
Reply to: