- Feature Articles
- CodeSOD
- Error'd
-
Forums
-
Other Articles
- Random Article
- Alex's Soapbox
- Announcements
- Best of…
- Best of Email
- Best of the Sidebar
- Bring Your Own Code
- Coded Smorgasbord
- Mandatory Fun Day
- Off Topic
- Representative Line
- News Roundup
- Editor's Soapbox
- Software on the Rocks
- Souvenir Potpourri
- Sponsor Post
- Tales from the Interview
- The Daily WTF: Live
- Virtudyne
Admin
Did they need to write a certain amount of tests to get management approval?
There's the genius that writes a tool to automatically write tests in order to maximize a questionable productivity metric, but there are also clueless idiots who genuinely believe tests like the ones in the article add value.
Admin
So you might say they stubbed their toes?
Admin
I dont' nedd nos speofelchekcn or pewoeriviw beucaue i nwiow hwo owt tyep!!
Submut
Admin
ArrayList is a very important library function, and it's good to know it is being thoroughly tested.
Admin
Stubbing dependencies: good Stubbing the class you're actually testing: not so good
Admin
I wonder what kind of input they fed the unit tests !
Admin
I suspect: N, S, F, W, true, false, FILE_NOT_FOUND.
Possibly also "nulla", the captcha.
Admin
On the bright side, tests will never break when someone modify the DAO's.
Admin
A wonderful WTF indeed! Looks as if the coder has no real knowledge whatsoever about what he is doing.
So he knows about stubbing. Great. But he does not know (or does not care) that what you stub in such a case is Connection and PreparedStatement so that "ps.executeQuery();" delivers a suitable List depending on the statement in the sql field.
Connection and PreparedStatement are both interfaces, Java really makes things easy for you here.
But the reason they started to stub at all is a wtf in it's own right: "the developers said that the tests took too long to run when they hit the database" So, if the tests completed reasonably fast, they would have actually queried a database from their unit tests?
That shows once again that unit tests might look simple but are sometimes hard to get right.
Admin
Well this just again shows that just unit testing is not enough, you also need integration testing and should be careful when stubbing dependencies.
Admin
Its called mocking, and its used extensively in all unit tests.
And no, proponents of unit test don't like it when you use the term in its original meaning when referring to unit testing.
Admin
Garbage In, Gospel Out
Admin
The test in the article does not add value, indeed. But that's because it has been implemented in a wrong way.
A correct implementation would have stubbed Connection and PreparedStatement to match any sql statements that might occur in the DAOs. That way, you could get your 100% code coverage of the DAOs and, for instance, would catch any changes to the sql strings (like an accidentally inserted letter or something like that).
Admin
Unit Tests don't magically transform bad programmers into good programmers. Programmers who take the time to actually understand what they are doing can use unit tests to verify the code still works the way they think it should.
Admin
"When queried, the developers said that the tests took too long to run when they hit the database, so they created stub-DAO's to return canned data, because they assumed the SQL queries would be correct and didn't need testing."
So, what did they think would happen when they went to production and the transaction volume started to actually tax the resources of the back end?
Admin
It's true that unit tests should not run against your main database. Either you use a local test database, or you mock out the database access code.
Mocking out the DAOs by returning canned data is quite reasonable when testing the rest of the code (business logic, etc). The only problem is that is leaves a hole in your testing, because you haven't verified the DAOs themselves.
So TRWTF here is not measuring code coverage - which would have shown up 'the DAO code never runs as part of the tests'.
Admin
Tests are testing the code. Good. But what's testing the tests? I demand tests written to test the tests!
Admin
bool trueness = true; if (trueness == trueness) { return true; } else { return trueness; }
Admin
Admin
Admin
I don't have experience with the PreparedStatement interface, so I may be missing something here, but in order to stub it realistically - and actually test the SQL as written, instead of simply returning canned data from one layer down - wouldn't you effectively need to write an SQL interpreter? Which is itself a large body of code, warranting its own tests...
A stub PreparedStatement interface returning a canned ResultSet still doesn't address the critical failure here: that the SQL statements in the code weren't tested against the target RDBMS. Even a very full-featured stub, wrapping a "real" DB such as SQLite backed by a local file, doesn't test the SQL dialect to be used in production against the RDBMS to be used in production.
Perhaps unit testing is the wrong approach for a DAO, and it really does need to be integration tested in conjunction with a test DB instance.
Admin
In cases where variables play a role (e.g. in the WHERE clause), you either compare by regex (more generic apporach) or you test against a few literal versions of the DAO's sql statement with the variable value hardcoded in.
You don't really need that much tests against a DAO - one or two that actually deliver data, one that doesn't and one that would produce an sql error. After all, you are testing the DAO here, not the database itself.
So in the end, your PreparedStatement will feature a long if..else cascade, but that's ok, as long as each block has a comment stating to which DAO the block belongs.
Admin
That's not really any better than what is presented in the article. It still doesn't address the issue that the SQL statements themselves may be incorrect. OK, so you are now testing that the DAO can correctly extract data from the ResultSet into its return type, which is meaningful - but you're still force-feeding it the correct ResultSet.
Arguably that may suffice for a unit test, but it still makes me cringe, and I still say in a real-world scenario there would need to be integration testing against the target RDBMS.
Admin
Admin
Fast testing theorem: Presumption of correctness approaches infinity as testing approaches zero.
Admin
I'd say the TRWTF is to presume that "Unit tests are OK" means "This code works correctly".
Unit-testing is a very nice feature (if used correctly). But it doesn't replace a regular test. It's too easy to "stub out" some complex feature that suddenly behaves completely different in a complex environment (Let's talk about multi-threading, deadlocks and performance issues...)
Admin
If you mock PreparedStatement, however you test the full DAO including the while loop in its getData() method. Especially the latter is important, since it populates getData()'s return value.
A mocked PreparedStatement provides the possibility to detect syntax errors in the DAO's sql statement like an accidentally inserted letter or something, and it allows you to detect intentional changes as well. The mocked PreparedStatement so serves as the reference for the sql statements. You change them just for the fun of it, your unit tests croak. Testing sql statements against their database schemata is somewhat beyond the scope of unit tests, but if someone provides a validateStmntAgainstSchema(stmnt, schema) function even that could be made a unit test. It doesn't matter what data the ResultSet contains, as long as it does contain data.This is a unit test of the DAO, not a test of the database or of what it contains.
Some typical test cases would be:
- does getData() return a List the same size as the ResultSet delivered by my mock PreparedStatement?
- does getData() return an empty List if ps.executeQuery() returns an empty ResultSet?
- does the DAO throw an exception if con.prepareStatement(sql) or ps.executeQuery() throw one?
And that's what this article is all about: a badly implemented unit test that gives a false sense of security, and a missing functional test against a real database before deployment.Unit tests are not a replacement for functional tests and even less for user acceptance tests; they are mainly meant for developers to get instant feedback the moment they break something.
Admin
The problem here is just lack of knowledge from whoever wrote those tests.
1st - When you're connecting to an external API, you write an adapter. You mock that adapter in your unit tests, and test the adapter with integration tests. Here they did neither.
2nd - You don't mock your unit under test, that's not called testing, it's called wasting time. When you do this you're testing nothing, your code is not even running you're just testing the mocks themselves, you're clearing doing a favour to the author of the mocking library, by making sure his mocks work.
Admin
Admin
For the most part I think this approach is wrong and add very little value.
You could have a valid but complicated SQL statement. Let's say it's running fine, but slow. You have a unit test verifying it. You go in and optimize the query. And then...
If you have a mocked PreparedStatement that returns a mocked ResultSet based on the SQL and parameters passed in... your test is failing!! Even if the query is running correctly. You get the same failure if you insert a typo into the query, as if you optimized the query. Sure, you could then update the test, but that's a lot of work. And the unit test is still not giving you the information you need, which is whether the new SQL query is still returning valid data.
I think the only way to test DAO is against a real database that is kept in a known state.
Admin
If signatures or return values of your object's methods change, you will have to update you unit tests. Fullstop.
"but that's a lot of work" is a lousy argument. "Did you test that release?" "No, too much work." "Ok, doesn't matter, it's just customer data we are loosing if anything goes wrong".
What is so difficult to understand about unit tests? Unit tests verify the basic behaviour of an object, nothing more. getData() should return a List the same size as the ResultSet returned by executeQuery(). What data is in the list is irrelevant (for now). Than you should perhaps think again or dig a bit deeper into what unit tests are, and what not.Of course you should also test your DAO against a decent database. But not as part of a suite of unit tests.
Here is a somewhat ideal testing scenario: Unit tests: on every submit Functional tests: if possible automated on a nightly build. Application-as-a-whole test: by your human tester as soon as he can test something. Say, pre-alpha. User acceptance test: if you work for a customer done by that customer once you are confident your software will pass the UAT.
Admin
It's no substitute for performance or integration testing, but it lets you actually run your query without a real database.
Admin
My eyes glazed over while reading, and I didn't really see anything WTF-worthy. I figured it was either a bad WTF or I'm TRWTF. Then I saw the last few lines and almost got a concussion myself from the ensuing headdesk moment.
Admin
Database DAO DOA.
Admin
I'm going to go out on a limb and guess that the other "unit tests" in the system were hitting the full stack. Some otherwise-smart guy tasked with making them run faster replaced all the database queries via some find and replace magic. Didn't pay too much attention to the fact that he was negating some tests. I've seen that one happen before.
Admin
It also kicks the door open 10 times wider for the good old fashion "not a representative data set fail"
Admin
Unit tests ARE wonderful, unless all the units are different.
My example: My brother generated MAC address roms for some vendors Ethernet cards. Now these MAC addresses MUST be unique, so he was THE vendor of the particular part (it was something like a $.30 chip, but programmed it went up to someting around $2.00 or so). The contract assembler of the Ethernet boards was also in charge of parts procurement and wanted to get good prices. They ignored the "sole source" notation on the MAC address chip, and just looked under the label and copied the same data into all of the chips for all of the boards. The end result was that all the Ethernet boards had the SAME MAC address. This is a REAL BAD thing. Of course they all individually passed unit tests.
Then the seller of the interface boards (the ones that had their logo on the board) came to my brother and wanted to read him the riot act about not doing his job, when he noted that the label on the MAC address chip wasn't his. The vendor rep thankfully did the right thing and picked up the phone then and there and put a stop work and "you're fired" call into the contract vendor.
It started out being a WTF, but somebody actually "fixed" the problem the proper way.
We can only hope that others attempt to follow the example.
Admin
Another example of "let the junior guy write the tests - it'll be good training". Feh.
CAPTCHA: wisi The tests all executed perfectly, and wisi fast!
Admin
The code is testing the tests...
The circle of test is maintained.
Admin
This.
Admin
The real WTF is assuming that because it passed the unit tests, that's it good to go.
I assume they caught this in integration testing but if they really just deployed it because it passed unit tests, THAT is the real WTF.
At my work, we HAVE to stub out DAOs in a manning somewhat similar (though not exactly) to the OP's code because the machines that automate the unit tests cannot connect to any databases.
In fact, unit tests are SUPPOSED to be self contained so that they do not require a database connection to function.
The real WTF is the trend of the industry to assume that unit testing is the Silver Bullet we've been searching for for so long. It's not. Useful, yes, panacea, no.
Admin
Well you got me, it's not really unit testing, it's integration testing. But it's being tested, normally as it's even developed (TDD). So if it's covered, I'm not too concerned if it's run with the unit test suite or the integration test suite. In most of my projects they run together.
Yes, it is too much work updating my mock database system as needed by the DAO (mock PreparedStatement, ResultSet), for the value I would get out of it. Testing that given a ResultSet with 10 items, I get a List with 10 items, is not important for me if the String with the SQL has an error and would return an exception. I would rather check the SQL and the data extraction at once, since they belong to the same unit (the DAO). This is my opinion, but going crazy with mocking to unit test components like this, leads to loosing focus on getting the DAO to run against a real database.
Here's how my testing looks:
Unit Testing: Mostly Services and utility classes (for example ResultSetExtractor and RowMappers) with mocked dependencies. This runs after every commit. (White box) Integration Testing: Real Services and Real DAO against a pristine database with the latest schema. This runs after every commit. (Black box) Integration Testing: A different project that tests the full application. Runs periodically, depending on the project. Human testing as appropriate.
Of course YMMV
Admin
"Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it." --Brian Kernighan
The same is probably true of writing unit tests for your code.
Admin
Seconded.
This is what I was getting at originally. No stub PreparedStatement will verify that the SQL is correct unless it actually attempts to execute that SQL. So either you need to move to integration testing against a real DB, or use a local file-backed/in-memory DB which still actually executes the SQL, but operates on known, canned data without requiring network access.
faoileag, your suggestions are continually missing the point, which is that the SQL statements are a core part of the DAO's functionality, and they may be incorrect. You can't test the DAO by doing a string compare on "expected SQL statement" and returning "expected results" and then claim the DAO works, because you haven't actually verified that the SQL statement on a real database would return those results - you've just hard-coded the assumption into your test harness!
(Sorry for over-using bold text, but my point is clearly not getting through, despite being understood by at least two others here.)
Admin
That's too bad. The Cargo Cult Of The Almighty Infallible Unit Test practically makes a mockery of itself with no outside help needed!
This is meant as a joke, but there's a lot of truth to it. It's tests all the way down.
Remember, folks, if all your unit tests pass, this means that all of your unit tests pass, nothing more, nothing less.
Admin
In most cases where the SQL varies based on variables, it's because the developer is embedding those variables into the SQL string, instead of passing them as parameters to the prepared statement, opening the door for an injection attack. As such, this should be regarded as a shooting offense...
Admin
Or for another one, null handling. ResultSet.getInt() (and getLong, getDouble, etc) returns a primitive rather than an object, and so returns 0 for a null column. If this is undesirable behaviour (and it usually is), you should be testing that the code has correctly distinguished between a real 0 and a null (via ResultSet.wasNull()).
Admin
Admin
Basically what you do is produce lots of variants of your program ("mutants") by doing minor tweaks like replace "<" with "<=" or replace "1" with "0" or some higher-level things like replace the use of one concrete class with another. The assumption (and one problem with mutation testing is that this assumption doesn't always hold) is that the new program is (1) functionally different from the original, and (2) incorrect -- and thus there should be a test which detects that incorrectness. So you compile and test each mutant. If one of your tests passes on the baseline program but fails on the mutant, then that's good (you've "killed the mutant" -- I'm not making up that terminology), and if a mutant makes it through the test suite with everything passes than that indicates that your test suite has a hole in it.
Admin
Add regression testing and it can go completely wonky. It's going to take me half a week to test a program after deleting functionality.