34

I am working with the following system:

Network Data Feed -> Third Party Nio Library -> My Objects via adapter pattern

We recently had an issue where I updated the version of the library I was using, which, among other things, caused timestamps (which the third party library returns as long), to be changed from milliseconds after the epoch to nanoseconds after the epoch.

The Problem:

If I write tests that mock the third party library's objects, my test will be wrong if I have made a mistake about the third party library's objects. For example, I didn't realize that the timestamps changed precision, which resulted in a need for change in the unit test, because my mock returned the wrong data. This is not a bug in the library, it happened because I missed something in the documentation.

The problem is, I cannot be sure about the data contained in these data structures because I cannot generate real ones without a real data feed. These objects are big and complicated and have a lot of different pieces of data in them. The documentation for the third party library is poor.

The Question:

How can I set up my tests to test this behavior? I'm not sure I can solve this issue in a unit test, because the test itself can easily be wrong. Additionally, the integrated system is large and complicated and it's easy to miss something. For example, in the situation above, I had correctly adjusted the timestamp handling in several places, but I missed one of them. The system seemed to be doing mostly the right things in my integration test, but when I deployed it to production (which has a lot more data), the problem became obvious.

I do not have a process for my integration tests right now. Testing is essentially: try to keep the unit tests good, add more tests when things break, then deploy to my test server and make sure things seem sane, then deploy to production. This timestamp issue passed the unit tests because the mocks were created wrong, then it passed the integration test because it didn't cause any immediate, obvious problems. I do not have a QA department.

  • 3
    Can your "record" a real data feed and "play" it later into the third party library? – Idan Arye Oct 6 '15 at 15:57
  • 2
    Someone could write a book on problems like this. In fact, Michael Feathers did write just that book: c2.com/cgi/wiki?WorkingEffectivelyWithLegacyCode In it, he describes a number of techniques for breaking difficult dependencies so that code can become more testable. – cbojar Oct 6 '15 at 16:52
  • 2
    The adapter around the third party library? Yes, that's exactly what I recommend. Those unit tests won't improve your code. They won't make it more reliable or more maintainable. You're just partially duplicating someone else's code at that point; in this case, you're duplicating some poorly written code from the sound of it. That's a net loss. Some of the answers suggest doing some integration testing; that's a good idea if you just want an, "Is this working?" sanity check. Good testing is hard, and it takes just as much skill and intuition as good code. – jpmc26 Oct 7 '15 at 0:15
  • 4
    A perfect illustration of the evil of built-ins. Why does not the library return a Timestamp class (containing any representation they want) and provide named methods (.seconds(), .milliseconds(), .microseconds(), .nanoseconds()) and of course named constructors. Then there would have been no issues. – Matthieu M. Oct 7 '15 at 8:41
  • 2
    The saying "all problems in coding can be solved by a layer of indirection (except, of course, the problem of too many layers of indirection)" comes to mind here.. – Dan Pantry Oct 7 '15 at 10:54
27

It sounds like you're already doing due diligence. But ...

At the most practical level, always include a good handful of both "full-loop" integration tests in your suite for your own code, and write more assertions than you think you need. In particular, you should have a handful of tests that perform a full create-read-[do_stuff]-validate cycle.

[TestMethod]
public void MyFormatter_FormatsTimesCorrectly() {

  // this test isn't necessarily about the stream or the external interpreter.
  // but ... we depend on them working how we think they work:
  var stream = new StreamThingy();
  var interpreter = new InterpreterThingy(stream);
  stream.Write("id-123, some description, 12345");

  // this is what you're actually testing. but, it'll also hiccup
  // if your 3rd party dependencies introduce a breaking change.
  var formatter = new MyFormatter(interpreter);
  var line = formatter.getLine();
  Assert.equal(
    "some description took 123.45 seconds to complete (id-123)", line
  );
}

And it sounds like you're already doing this sort of thing. You're just dealing with a flaky and/or complicated library. And in that case, it's good to throw in a few "this is how the library works" types of tests that both verify your understanding of the library and serve as examples of how to use the library.

Suppose you need to understand and depend on how a JSON parser interprets each "type" in a JSON string. It's helpful and trivial to include something like this in your suite:

[TestMethod]
public void JSONParser_InterpretsTypesAsExpected() {
  String datastream = "{nbr:11,str:"22",nll:null,udf:undefined}";
  var o = (new JSONParser()).parse(datastream);

  Assert.equal(11, o.nbr);
  Assert.equal(Int32.getType(), o.nbr.getType());
  Assert.equal("22", o.str);
  Assert.equal(null, o.nll);
  Assert.equal(Object.getType(), o.nll.getType());
  Assert.isFalse(o.KeyExists(udf));
}

But secondly, remember that automated testing of any kind, and at almost any level of rigor, will still fail to protect you against all bugs. It's perfectly common to add tests as you discover problems. Not having a QA department, this means a lot of those problems will be discovered by end-users.

And to a significant degree, that's just normal.

And thirdly, when a library changes the meaning of a return-value or field without renaming the field or method or otherwise "breaking" dependent code (maybe by changing its type), I'd be pretty damn unhappy with that publisher. And I'd argue that, even though you should probably have read the changelog if there is one, you should probably also pass some of your stress onto the publisher. I'd argue they need the hopefully-constructive criticism ...

  • Ugh, I wish it was as simple as feeding a json string into the library. It's not. I cannot do the equivalent of (new JSONParser()).parse(datastream), as they grab the data directly from a NetworkInterface and all the classes that do the actual parsing are package private and proguarded. – durron597 Oct 6 '15 at 17:09
  • Also, the changelog did not include the fact that they changed timestamps from ms to ns, among the other headaches they did not document. Yes, I am very unhappy with them, and I have expressed this to them. – durron597 Oct 6 '15 at 17:10
  • @durron597 Oh, it almost never is. But, you can often fake the underlying data source -- like in the first code example. ... Point is: do full-loop integration tests when possible, test your understanding of the library when possible, and just be aware that you will still let bugs into the wild. And your 3rd party vendors must be accountable for making invisible, breaking changes. – svidgen Oct 6 '15 at 17:11
  • @durron597 I'm not familiar with the NetworkInterface ... is it something you can feed data into by connecting the interface to a port on localhost or something? – svidgen Oct 6 '15 at 17:14
  • NetworkInterface. It's a low level object for directly working with a network card and opening sockets on it, etc. – durron597 Oct 6 '15 at 17:15
11

Short answer: It's hard. You're probably feeling like there are no good answers, and that's because there are no easy answers.

Long answer: Like @ptyx says, you need system tests and integration tests as well as unit tests:

  • Unit tests are fast and easy to run. They catch bugs in individual sections of code and use mocks to make running them possible. By necessity, they can't catch mismatches between pieces of code (like milliseconds versus nanoseconds).
  • Integration tests and system tests are slow(er) and hard(er) to run but catch more errors.

Some specific suggestions:

  • There's some benefit to simply getting a system test to run as much of the system as possible. Even if it can't validate very much of the behavior or do very well at pinpointing the problem. (Micheal Feathers discusses this more in Working Effectively with Legacy Code.)
  • Investing in testability helps. There are a huge number of techniques you can use here: continuous integration, scripts, VMs, tools to play back, proxy, or redirect network traffic.
  • One of the advantages (for me, at least) of investing in testability may be non-obvious: if tests are tedious, annoying, or cumbersome to write or run, then it's too easy for me to simply skip them if I'm pressured or tired. Keeping your tests below the "It's so easy that there's no excuse to not do this" threshold is important.
  • Perfect software is not feasible. Like everything else, effort spent on testing is a tradeoff, and sometimes it's not worth the effort. Constraints (such as your lack of a QA department) exist. Accept that bugs will happen, recover, and learn.

I've seen programming described as the activity of learning about a problem and solution space. Getting everything perfect ahead of time may not be feasible, but you can learn after the fact. ("I fixed timestamp handling in several places but missed one. Can I change my data types or classes to make timestamp handling more explicit and harder to miss, or to make it more centralized so I only have one place to change? Can I modify my tests to verify more aspects of the timestamp handling? Can I simplify my test environment to make this easier in the future? Can I imagine some tool that would have made this easier, and if so, can I find such a tool on Google?" Etc.)

7

I updated the version of the library … which … caused timestamps (which the third party library returns as long), to be changed from milliseconds after the epoch to nanoseconds after the epoch.

This is not a bug in the library

I strongly disagree with you here. It is a bug in the library, a rather insidious one in fact. They have changed the semantic type of the return value, but did not change the programmatic type of the return value. This can wreak all kinds of havoc, especially if this was a minor version bump, but even also if it was a major one.

Let's say instead the library returned a type of MillisecondsSinceEpoch, a simple wrapper that holds a long. When they changed it to a NanosecondsSinceEpoch value, your code would have failed to compile, and would have obviously pointed you to the places where you need to make changes. The change couldn't silently corrupt your program.

Better yet would be a TimeSinceEpoch object that could adapt it's interface as more precision was added, such as adding a #toLongNanoseconds method along side the #toLongMilliseconds method, requiring no change to your code whatsoever.

The next problem is that you don't have a reliable set of integration tests to the library. You should write those. Better would be to create an interface around that library to encapsulate it away from the rest of your application. Several other answers address this (and more keep popping up as I type). Integration tests should be run less frequently than your unit tests. That is why having a buffer layer helps. Segregate your integration tests into a separate area (or name them differently) so you can run them as needed, but not every time that you run your unit test.

  • 2
    @durron597 I would still contend that it's a bug. Beyond the lack of documentation, why change the expected behavior at all? Why not a new method that provides the new precision, and let the old method still provide millis? And why not provide a way for the compiler to alert you through a change in return type? It doesn't take much to make this a lot clearer, not just in the documentation, but in the code itself. – cbojar Oct 6 '15 at 17:23
  • 1
    @gbjbaanb, “that they have poor release practices” seems like a bug to me – Arturo Torres Sánchez Oct 7 '15 at 13:53
  • 2
    @gbjbaanb A 3rd-party library [should] make a "contract" with its users. Breaking that contract -- whether it's documented or not -- can/should be regard as a bug. As others have said, if you must change something, add to the contract with a new function/method (cf. all the ...Ex() methods in the Win32API). If this isn't feasible, "breaking" the contract by renaming the function (or its return type) would have been better than altering the behaviour. – TripeHound Oct 7 '15 at 15:59
  • 1
    This is a bug in the library. Using nanoseconds in a long is pushing it. – Joshua Oct 7 '15 at 17:42
  • 1
    @gbjbaanb You say it is not a bug since it's intended behavior, even if unexpected. In that sense it is not an implementation bug, but it is a bug the same. It might be called a design defect or an interfacing bug. The faults lie in the fact that it exposes a primitive obsession with longs rather than explicit units, its abstraction is leaky as it exports details of its internal implementation (that the data is stored as a long of a certain unit), and that it violates the principle of least astonishment with a subtle unit change. – cbojar Oct 7 '15 at 17:42
5

You need integration and system tests.

Unit tests are great about verifying that your code behaves as you expect. As you realize, it does nothing to challenge your assumptions or ensure your expectations are sane.

Unless your product has little interaction with external systems, or interacts with systems so well known, stable and documented that they can be mocked with confidence (this rarely happens in the real world) - unit tests are not enough.

The higher level your tests are, the more they'll protect you against the unexpected. That comes at a cost (convenience, speed, brittleness...), so unit tests should remain the foundation of your testing, but you need other layers, including - eventually - a tiny bit of human testing which goes a long way at catching stupid things that nobody thought about.

2

The best would be to create a minimal prototype, and understand how the library works exactly. By doing that, you will gain some knowledge about the library with poor documentation. A prototype can be a minimalistic program that uses that library and does the functionality.

Otherwise, it makes no sense to write unit tests, with half-defined requirements and weak understanding of the system.

As for your specific problem - about using wrong metrics : I would treat it as a requirements change. Once you recognized the problem, change the unit tests and the code.

1

If you were using a popular, stable library, then you could perhaps assume that it won't play nasty tricks on you. But if things like what you described happen with this library, then obviously, this is not one. After this bad experience, each and every time something goes wrong in your interaction with this library, you will need to examine not only the possibility that you have made a mistake, but also, the possibility that the library may have made a mistake. So, let's say that this is a library that you are "unsure" about.

One of the techniques employed with libraries that we are "unsure" about is to build an intermediate layer between our system and said libraries, which abstracts the functionality offered by the libraries, asserts that our expectations of the library are right, and also greatly simplifies our life in the future, should we decide to give that library the boot and replace it with another library that behaves better.

  • This doesn't really answer the question. I already do have a layer separating the library from my system, but the problem is that my abstraction layer can have "bugs" when the library changes on me without warning. – durron597 Oct 6 '15 at 17:06
  • 1
    @durron597 Then perhaps the layer is not sufficiently isolating the library from the rest of your application. If you are finding that you are having a hard time testing that layer, maybe you need to simplify the behavior and more strongly isolate the underlying data. – cbojar Oct 6 '15 at 17:18
  • What @cbojar said. Also, let me repeat something which may have gone unnoticed in the above text: The assert keyword (or function, or facility, depending on what language you are using,) is your friend. I am not talking about assertions in the unit/integration tests, I am saying that the isolation layer should be very heavy with assertions, asserting everything assertable about the behavior of the library. – Mike Nakis Oct 6 '15 at 17:35
  • These assertions do not necessarily execute on production runs, but they do execute while testing, having a white-box view of your isolation layer and therefore being able to make sure (as much as possible) that the information that your layer receives from the library is sound. – Mike Nakis Oct 6 '15 at 17:35

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.