WhyTF am I putting my Node in The Intern?

Yamikuronue

OK, my next issue:

WTF am I supposed to do about ETXTBSY, text file is busy 'unitTest.db' in my destroy function:

destroy: function(callback) {
	db.close(callback);
}

I'm trying to close it after each test, so I can delete the DB when it's done closing.

accalia

callback hell that is...

hmm.... try this:

GitHub - caolan/async: Async utilities for node and the browser

Async utilities for node and the browser. Contribute to caolan/async development by creating an account on GitHub.

var async = require('async');
var tests ={
	insert: function () {
		async.waterfall([
		function(next){
			dao.createDB("unitTest.db");
			var deferred = this.async(10000);

			dao.getItems(deferred.rejectOnError(function(items){
				next(null, items);
			};
		},
		function(items, next){
			assert.isArray(items, "Items was not an array");
			assert.isTrue(items.length < 1,"Items had 1 or more items.");
			
			dao.addItem("Test", "test", 0, next);
		},
		function(next) {
			assert.isUndefined(err, "No error should occur.");
			dao.getItems(
				deferred.callback(
					function(items) {
						assert.isArray(items, "Items was not an array");
						assert.isTrue(items.length === 1,"Items did not contain one item.");
					}
				)
			);
		}]);
	}
};

i could probably unroll that some more and my naieve edit probably doesn't work, but you get the idea i hope?

Yamikuronue

I'm mostly flabberghasted that in a simple CRUD unit test I made it to callback hell... I've been working on this code for like, 3-4 hours all told. Jesus christ. It doesn't even DO anything yet.

Yamikuronue

This ETXTBSY may be related to Virtualbox and Vagrant. Because. Reasons.

accalia

it's node JS. it's all callback hell.

i use caolan's async library in pretty much every project i do because it just works and it reduces much of the insane callback nesting.

as to why your test DB is still busy it's probably a still uncompletes database thing. try adding a couple of seconds wait and see if it starts working (if it does then we can try to find where the still extant callback is and move on from there!)

accalia

@Yamikuronue said:

This ETXTBSY may be related to Virtualbox and Vagrant

aaah. virtual environment.... yeah odd things happen with virtualbox/vagrant.

not sure i have a good solution for you there....

Yamikuronue

It's apparently a Windows/Linux thing. Windows is aggressive about locking and thus doesn't allow low-level file operations to be reliable on shared folders.

Yamikuronue

Error: SQLITE_ERROR: table TodoItems already exists

Urhg whatever, comment out the line that recreates the database in the second test since clearly it didn't drop correctly at the end of the first one.

Error: SQLITE_ERROR: no such table: TodoItems

WTF? Put back the line:

Error: SQLITE_ERROR: table TodoItems already exists

Piece of shit.

Yamikuronue

Someone explain this please, maybe @accalia ?

		db.serialize(function() {
			db.run("DROP TABLE IF EXISTS TodoLists");
			db.run("CREATE TABLE TodoLists (listID INTEGER PRIMARY KEY, listName TEXT)");
	
			db.run("DROP TABLE IF EXISTS TodoItems");
			db.run("CREATE TABLE TodoItems (itemID INTEGER PRIMARY KEY, listID INTEGER NOT NULL, itemName TEXT, itemText TEXT, state INTEGER, FOREIGN KEY(listID) REFERENCES TodoLists(listID))");
		});

How on earth does that ever throw:

Error: SQLITE_ERROR: table TodoLists already exists

accalia

not sure. but i suspect your drop is failing for some reason.

try this:

	function errorprint(err){if(err) {console.error(arguments);}
	db.serialize(function() {
		db.run("DROP TABLE IF EXISTS TodoLists",errorprint);
		db.run("CREATE TABLE TodoLists (listID INTEGER PRIMARY KEY, listName TEXT)",errorprint);

		db.run("DROP TABLE IF EXISTS TodoItems",errorprint);
		db.run("CREATE TABLE TodoItems (itemID INTEGER PRIMARY KEY, listID INTEGER NOT NULL, itemName TEXT, itemText TEXT, state INTEGER, FOREIGN KEY(listID) REFERENCES TodoLists(listID))",errorprint);
	});

Yamikuronue

@accalia said:

errorprint

...well first it made the entire error go away in favor of an assertion error... so then I fixed more of my code... and now I have the spectacularly unhelpful:

PASS: main - todoItemTests - tests - createsDB (5ms)
PASS: main - todoItemTests - tests - createList (0ms)
{ '0': { [Error: SQLITE_ERROR: table TodoLists already exists] errno: 1, code: 'SQLITE_ERROR' } }

Which tells me it's the third test, the one with real logic in it, and which begins dao.createDB("/home/vagrant/unitTest.db");, which contains the drop-and-recreate code.

accalia

oh! that's in your DAO!

got it.

don't supply a callback to serialize.

do this instead:

function errorprint(err){if(err) {console.error(arguments);}
db.serialize();
db.run("DROP TABLE IF EXISTS TodoLists",errorprint);
db.run("CREATE TABLE TodoLists (listID INTEGER PRIMARY KEY, listName TEXT)",errorprint);
db.run("DROP TABLE IF EXISTS TodoItems",errorprint);
db.run("CREATE TABLE TodoItems (itemID INTEGER PRIMARY KEY, listID INTEGER NOT NULL, itemName TEXT, itemText TEXT, state INTEGER, FOREIGN KEY(listID) REFERENCES TodoLists(listID))",errorprint);

serialize with a callback applys the serialization to the commands in the callback. it does not make them synchronous. once the callback returns commands are once again parallelized.

if you want the thing to just stay in serialized mode them do as i did above and explicitly enter parralelized mode with db.parallelize when you need it.

Yamikuronue

		db.serialize();
			db.run("DROP TABLE IF EXISTS TodoItems",errorprint);
			db.run("DROP TABLE IF EXISTS TodoLists",errorprint);


			db.run("CREATE TABLE TodoLists (listID INTEGER PRIMARY KEY, listName TEXT)",errorprint);
			db.run("CREATE TABLE TodoItems (itemID INTEGER PRIMARY KEY, listID INTEGER NOT NULL, itemName TEXT, itemText TEXT, state INTEGER, FOREIGN KEY(listID) REFERENCES TodoLists(listID))",errorprint);
		db.parallelize();

{ '0': { [Error: SQLITE_ERROR: table TodoLists already exists] errno: 1, code: 'SQLITE_ERROR' } }
{ '0': { [Error: SQLITE_ERROR: table TodoItems already exists] errno: 1, code: 'SQLITE_ERROR' } }

Yamikuronue

@accalia said:

serialize with a callback applys the serialization to the commands in the callback. it does not make them synchronous

I'm not sure I understand this sentence.

I mean, if the lines run async, but the commands are queued up at the door waiting to be executed because the DB is in serial mode so it only executes one at a time... how is that not sync? Or does it mean they can get shuffled on their way into the queue? But if so, how does that differ when I don't put them in the callback?

I feel like this first comment from the docs lies to me:

db.serialize(function() {
  // These two queries will run sequentially.
  db.run("CREATE TABLE foo (num)");
  db.run("INSERT INTO foo VALUES (?)", 1, function() {
    // These queries will run in parallel and the second query will probably
    // fail because the table might not exist yet.
    db.run("CREATE TABLE bar (num)");
    db.run("INSERT INTO bar VALUES (?)", 1);
  });
});

This line in the docs indicates that either way should be synonymous:

If you call it without a function parameter, the execution mode setting is sticky and won't change until the next call to Database#parallelize.

cartman82

Perhaps these statements are run in some kind of transaction and you need something like "commit" or "go" or ";" between the first "DROP" and "CREATE"?

accalia

it's poorly worded. not lying.

think of it this way: db.serialize() turns on query serialization until further notice and db.serialize(function(){}) turns on serialization only for the function scope that is contained in the callback, and does not retain it being on for nested function scopes.

does that make more sense?

RaceProUK

DDL statements don't need a COMMIT, though depending on the DBMS, they may need a GO.

Yamikuronue

db.serialize();
	db.run("BEGIN TRANSACTION",errorprint);
	db.run("DROP TABLE IF EXISTS TodoItems",errorprint);
	db.run("DROP TABLE IF EXISTS TodoLists",errorprint);
	db.run("COMMIT TRANSACTION",errorprint);

	db.run("BEGIN TRANSACTION",errorprint);
	db.run("CREATE TABLE TodoLists (listID INTEGER PRIMARY KEY, listName TEXT)",errorprint);
	db.run("CREATE TABLE TodoItems (itemID INTEGER PRIMARY KEY, listID INTEGER NOT NULL, itemName TEXT, itemText TEXT, state INTEGER, FOREIGN KEY(listID) REFERENCES TodoLists(listID))",errorprint);
	db.run("COMMIT TRANSACTION",errorprint);
db.parallelize();

{ '0': { [Error: SQLITE_ERROR: table TodoLists already exists] errno: 1, code: 'SQLITE_ERROR' } }
{ '0': { [Error: SQLITE_ERROR: table TodoItems already exists] errno: 1, code: 'SQLITE_ERROR' } }

Yamikuronue

@accalia said:

db.serialize() turns on query serialization until further notice and db.serialize(function(){}) turns on serialization only for the function scope that is contained in the callback

I understood that part, the part I was calling a lie was

@Yamikuronue said:

// These two queries will run sequentially.

accalia

they will!

or they're supposed to....

accalia

that's..... that's not supposed to happen......

does it do that even when run on its own in just a test file?

Yamikuronue

Clearly they're not :(

boomzilla

Thank you for posting all this stuff, BTW. My issues seem less WTF now.

cartman82

There's something else happening here. I just did a quick node/sqlite project and the code like yours should work.
Are you sure there are no other things happening in parallel with this code?

EDIT
Perhaps the test is run twice? Or a different test is running at the same time?

ben_lubar

I can understand using callbacks for things like HTTP request handlers, but database queries?!

cartman82

@ben_lubar said:

I can understand using callbacks for things like HTTP request handlers, but database queries?!

What's the difference? You go to a distant server through TCP either way.

ben_lubar

The HTTP request handler is on the server, and the server doesn't know when it will be getting a request. the database query is on the client and the client knows exactly when it will send the query.

Once again, terminology won the argument and confused everyone.

mott555

@ben_lubar said:

I can understand using callbacks for things like HTTP request handlers, but database queries?!

Node is single-threaded, so the idea is to allow other requests to be processed while waiting for a database query to return. However, this can be stupid if you know nothing is going to happen in the interim, especially with SQLite when you're not even talking to another server for database access! Really callback hell is the only downside to Node IMO.

I use the async library a lot just like @Accalia recommends, it hides some of the ugliness of callback hell.

Yamikuronue

That could be -- mayhap the tests are running in parallel. The whole file as of an hour or so ago is here: https://github.com/yamikuronue/freezing-octo-cyril/blob/master/tests/unit/DAO/todoItem.js

ben_lubar

I can't imagine a nodejs server being terribly efficient if it can only access one CPU core and every request has to wait in line for the previous n - 1 requests.

mott555

The async nature makes nodejs extremely efficient even though it's single-threaded. But since you don't know when callbacks get called, a lot of tasks (as in series of functions and callbacks) get executed in an interleaved fashion.

cartman82

@ben_lubar said:

The HTTP request handler is on the server, and the server doesn't know when it will be getting a request. the database query is on the client and the client knows exactly when it will send the query.

In node land, what you're describing is usually called "listener" or "handler". The word callback is usually used for some kind of request you're making (called exactly once, once the task is done).

@Yamikuronue said:

That could be -- mayhap the tests are running in parallel. The whole file as of an hour or so ago is here: https://github.com/yamikuronue/freezing-octo-cyril/blob/master/tests/unit/DAO/todoItem.js

Ugh, what a mess. Where is the DROP / CREATE code?

cartman82

Looking through Intern docs a bit, I don't see any mention of whether tests are run in parallel or sequential. But there's a big accent on the usage on deferred object for async tests.

I think what's happening here is that all these tests are executed at the same time. So they start randomly creating, deleting, initializing the db file. Who knows where that DROP/CREATE part is called.

I suggest you change each test to immediately create a deferred object and return it synchronously. That should prevent the other tests from starting at the same time. Then create the db file. Then do the test. Then delete the file and resolve the deferred.

dkf

Is there a reason for not doing CREATE TABLE IF NOT EXISTS? It might also make sense to use a pure in-memory database for the majority of testing.

Yamikuronue

@cartman82 said:

Where is the DROP / CREATE code?

In the object under test, linked up-thread: https://github.com/yamikuronue/freezing-octo-cyril/blob/master/src/dao/todoItems.js

Yamikuronue

@dkf said:

Is there a reason for not doing CREATE TABLE IF NOT EXISTS?

At this point? Stubbornness. Well, and atomicity: when I write a test that inserts two rows, I'd rather not have it fail because it ran in a different order than the one with only one row inserted.

dkf

@Yamikuronue said:

At this point? Stubbornness. Well, and atomicity: when I write a test that inserts two rows, I'd rather not have it fail because it ran in a different order than the one with only one row inserted.

For unit testing, I'd just stick to in-memory DBs anyway, and use a completely new DB for each test. Overfussy perhaps, but might as well and then you're definitely starting from a known state. I'd try to avoid hitting the hardware for anything less than an integration test; it's not like the DB itself is untested.

Yamikuronue

See, I figured I'd do that with sqlite - delete the file between each test.

Then it barfed all over everything and now I'm at home doing other things.

izzion

Since I've already got one stupid question/statement in this thread that I've deleted, I might as well go for the double...

Is it possible that the calls from Intern to the DAO are referencing different instances of the DAO, such that dao.db doesn't exist when dao.addItems is called? I see the dao.getItems method doing a check to see if db exists, but I don't see the same check in dao.addItems.

Yamikuronue

@izzion said:

Is it possible that the calls from Intern to the DAO are referencing different instances of the DAO

uuuummmmm..... That's.... a good question.

I think.... because of CommonJS... no? I think it's one instance that's created when I do require at the top of the file? Don't quote me on that though.

cartman82

Yes, require returns singletons.

Monarch

May be not the best idea to mess with the raw sqlite database on the file level.
Even though the way you do it is likely the way I would have done it I suggest that you diverge a little:

Move the checks and operations to the database logic

use a SQL DROP statement on all the tables instead of unlink the database file
Don't check if the database exists by checking if the file exists instead
change your CREATE TABLE statement to use IF NOT EXISTS.

combining TABLE DROP and create table if not exists will provide the same logic. and eliminate the need for the lower level file checks. it will require a little bit extra work to write and maintain drop statement for each table the requires it on cleanup.

dkf

@Yamikuronue said:

See, I figured I'd do that with sqlite - delete the file between each test.

Unless the test requires that the DB be on disk (which some tests probably will care about, but most not) it's better to use a pure in-memory DB for testing. It removes the dependency on the filesystem('s detailed semantics). You can create an in-memory database by using the special “filename” :memory:; that's the only difference. It's guaranteed to only live as long as the “connection” does.

Doing the DDL in more than one transaction worries me quite a bit. My shoulder blades itch in a way that makes me think it's asking for trouble. :)

Yamikuronue

@dkf said:

using the special “filename” :memory:;

That's exactly what I need! Thanks!

I was trying to

@Monarch said:

use a SQL DROP statement on all the tables instead of unlink the database file

but that's when I ended up with serialization issues.

Yamikuronue

Any idea why inserting a row that violates my foreign key constraint doesn't throw an error?

Create statements:

CREATE TABLE TodoLists (
    listID INTEGER PRIMARY KEY, 
    listName TEXT
)

CREATE TABLE TodoItems (
     itemID INTEGER PRIMARY KEY, 
     listID INTEGER NOT NULL, 
     itemName TEXT, 
     itemText TEXT, 
     state INTEGER, 
     FOREIGN KEY(listID) REFERENCES TodoLists(listID)
)

Insert:

INSERT INTO TodoItems (listID, itemName, itemText, state) VALUES (?,?,?,?)

Run with:

dao.addItem(12, "Test", "test", 0, function() {/*snip*/})
//Signature: addItem: function(listID, name, text, state, callback)

//Inside addItem:
stmt.run(listID, name, text, state, function(err) { /*snip*/});

As the only statement after creating the database, so there can't be any lists yet.

err is coming up null in the callback

VinDuv

According to the documentation, foreign keys constraints need to be explicitely enabled when opening the database:

SQLite Foreign Key Support

Maybe that’s the problem?

Yamikuronue

...wat.

http://img.pandawhale.com/39003-wat-know-your-meme-gif-olJb.gif

Okay, going to enable the foreign key constraints I spent time crafting because obvs I wrote them for shits and giggles right? >.>

RaceProUK

@VinDuv said:

foreign keys constraints need to be explicitly enabled when opening the database

I… have the perfect image:

Oh, were you expecting a pink hedgehog? Sorry to disappoint

VinDuv

@Yamikuronue said:

I wrote them for shits and giggles right?

You’re using a database which ignores column data types. You should probably assume that most constraint checks are nonexistent or disabled by default.

Yamikuronue

Yeah I'm beginning to think TRWTF was Sqlite. I figured I'd use something lightweight and simple and I think I went TOO lightweight.

The actual applications we're going to be writing in Node will use MSSQL :/ I should have just used MySQL and dealt with it