Skip to content

Commit

Permalink
Fix knexfile resolution. Add missing test (#2923)
Browse files Browse the repository at this point in the history
* Fix knexfile resolution. Add missing test

* Try fixing Jake test execution

* Avoid having non-test files in jake folder

* Fix test compatibility with Node 6

* Fix the fix
  • Loading branch information
kibertoad committed Nov 22, 2018
1 parent 7a6e817 commit 461868b
Show file tree
Hide file tree
Showing 6 changed files with 102 additions and 5 deletions.
10 changes: 6 additions & 4 deletions bin/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ function mkConfigObj(opts) {
function initKnex(env, opts) {
checkLocalModule(env);

if (!env.pathToKnexFile) {
if (!opts.knexfile) {
if (opts.client) {
env.configuration = mkConfigObj(opts);
} else {
Expand All @@ -67,7 +67,7 @@ function initKnex(env, opts) {
}
// If knexfile is specified
else {
env.configuration = require(env.pathToKnexFile);
env.configuration = require(opts.knexfile);
}

if (process.cwd() !== env.cwd) {
Expand Down Expand Up @@ -97,7 +97,10 @@ function initKnex(env, opts) {
process.exit(1);
}

if (argv.debug !== undefined) config.debug = argv.debug;
if (argv.debug !== undefined) {
config.debug = argv.debug;
}

const knex = require(env.modulePath);
return knex(config);
}
Expand Down Expand Up @@ -297,7 +300,6 @@ cli.on('requireFail', function(name) {
cli.launch(
{
cwd: argv.cwd,
pathToKnexFile: argv.knexfile,
require: argv.require,
completion: argv.completion,
},
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
"chai": "^4.2.0",
"chai-subset-in-order": "^2.1.2",
"coveralls": "^3.0.2",
"cross-env": "^5.2.0",
"eslint": "5.9.0",
"eslint-config-prettier": "^3.3.0",
"eslint-plugin-import": "^2.14.0",
Expand Down Expand Up @@ -86,7 +87,7 @@
"plaintest": "mocha --exit -t 10000 -b -R spec test/index.js && npm run tape",
"prepublish": "npm run babel",
"pre_test": "npm run lint",
"bin_test": "jake -f test/jake/*",
"bin_test": "cross-env KNEX_PATH=../knex.js KNEX=bin/cli.js jake -f test/jake/*",
"tape": "node test/tape/index.js | tap-spec",
"debug_tape": "node --inspect-brk test/tape/index.js",
"test": "npm run pre_test && nyc mocha --exit --check-leaks --globals __core-js_shared__ -t 10000 -R spec test/index.js && npm run tape && npm run bin_test",
Expand Down
53 changes: 53 additions & 0 deletions test/jake-util/helpers/migrationtesthelper.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/* eslint-disable no-undef */
/* eslint-disable no-console */

const os = require('os');
const fs = require('fs');
const rimrafSync = require('rimraf').sync;

function assertExec(cmd, desc) {
desc = desc || 'Run ' + cmd;
return new Promise((resolve, reject) => {
let stderr = '';
console.log(`Executing: ${cmd}`);
const bin = jake.createExec([cmd]);
bin.addListener('error', (msg, code) =>
reject(Error(desc + ' FAIL. ' + stderr))
);
bin.addListener('cmdEnd', resolve);
bin.addListener('stderr', (data) => (stderr += data.toString()));
bin.run();
});
}

function test(taskList, description, func) {
const tmpDirPath = os.tmpdir() + '/knex-test-';
rimrafSync(tmpDirPath);
const tempFolder = fs.mkdtempSync(tmpDirPath);
desc(description);
const taskName = description.replace(/[^a-z0-9]/g, '');
taskList.push(taskName);
task(taskName, { async: true }, () => {
let itFails = false;
func(tempFolder)
.then(() => {
console.log('☑ ' + description);
})
.catch((err) => {
console.log('☒ ' + err.message);
itFails = true;
})
.then(() => {
rimrafSync(tmpDirPath);
rimrafSync('test/jake/test.sqlite3');
if (itFails) {
process.exit(1);
}
});
});
}

module.exports = {
assertExec,
test,
};
9 changes: 9 additions & 0 deletions test/jake-util/knexfile/knexfile.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
module.exports = {
client: 'sqlite3',
connection: {
filename: __dirname + '/../test.sqlite3',
},
migrations: {
directory: __dirname + '/../knexfile_migrations',
},
};
9 changes: 9 additions & 0 deletions test/jake-util/knexfile_migrations/simple_migration.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
exports.up = (knex) => {
return knex.schema.createTable('rules', (table) => {
table.string('name');
});
};

exports.down = (knex) => {
return knex.schema.dropTable('rules');
};
23 changes: 23 additions & 0 deletions test/jake/knexfile.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
#!/usr/bin/env jake
'use strict';
/* eslint-disable no-undef */
/* eslint-disable no-console */

const path = require('path');
const {
assertExec,
test,
} = require('../jake-util/helpers/migrationtesthelper');

const KNEX = path.normalize(__dirname + '/../../bin/cli.js');

const taskList = [];
/* * * TESTS * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */

test(taskList, 'Run migrations with knexfile passed', (temp) => {
return assertExec(
`node ${KNEX} migrate:latest --knexfile=../test/jake-util/knexfile/knexfile.js`
);
});

task('default', taskList);

4 comments on commit 461868b

@joselcvarela
Copy link

Choose a reason for hiding this comment

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

I am having problems with knexfile with version "knex": "^0.16.0-next5"
In version 0.15.2 I just ran "npx knex migrate:latest" inside folder containing knexfile and everything works like a charm. But now, the same command tells that I need to pass knexfile argument, and when I did it, tells me that could't find that "require".
So I debugged and I find out that knexfile is resolving inside node_modules directory.
I am running on Windows 10.

This is my lauch.json I used with VS Code to debugging

{
    {
      "type": "node",
      "request": "launch",
      "name": "KNEX Migrate",
      "program": "${workspaceRoot}\\node_modules\\knex\\bin\\cli.js",
      "cwd": "${workspaceFolder}",
      "args": [
        "--knexfile=/src/database/knexfile.js",
        "migrate:make",
        "create-essential-indexes"
      ]
    }
  ]
}

@kibertoad can you help?

@kibertoad
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@joselcvarela Thank you for reproduction. This is a regression in next5, I hope to fix it tonight and then we will be able to release stable 0.16.0 soon.

@kibertoad
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@joselcvarela Could you try with the code from #2935 ? I hope I fixed the issue, would appreciate if you could test it.

@kibertoad
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@joselcvarela Please try 0.16.1-next1 - should resolve the issue.

Please sign in to comment.