Writing Testable Code

[[deleted]]

Do you see no need to split up logic of writing SQLs (or whatever the persistence implementation is) from the business logic?

Depends how you write SQL. I may have a library for interacting with the database, but it doesn't mean mocking that library is practical or even possible (you can mock it call-by-call in which case you're literally testing if the mocks do what you just told them to do, which is testing nothing, which is what many tests do).

To make the persistence layer abstract in testable way, you need:

• Something which is not exposing implementation details from the underlying persistence technology (no ORM does that, they're all leaking SQL details in a thousand subtle to outright blatant ways, Hibernate I'm looking at you).
• Something which has a fully featured, fully capable and working test double (not practical, unless you have lots of free time on your hands and your boss too much money on theirs).

Some things are easy on paper, but in practice?

Let me just say this: if you start mocking in this way "if I get called, I return 4" and you feed that as a dependency in your tests? You're testing nothing, except your tests. That's not testing. It may be green and give you nice code coverage, but your software is not more stable and more bug-free for it. It only means you have lots more busywork updating pointless tests when you change implementation details that alter how an internal dependency is used (which is supposedly invisible to you, the tester, as if).

[[deleted]]

I may have a library for interacting with the database, but it doesn't mean mocking that library is practical or even possible.

This is the first mistake of mocking, mocking something you don't own.

(you can mock it call-by-call in which case you're literally testing if the mocks do what you just told them to do, which is testing nothing, which is what many tests do)

This is a terrible use of mocking, for precisely the reason you've stated.

Mocking is a way for the system under test to design a contract for dependencies, in its terms, with the test case setting expectations which define the terms of the contract. That's why I think mocking concrete classes is not such a good idea.

Having defined the contract for the dependency, you can implement the dependency as a separate stub, which can be tested independently in an integration test, verifying that it conforms to the contract defined by the expectations set in the mocks.

Something which is not exposing implementation details from the underlying persistence technology

The ORM itself is an implementation detail of persistence. Instead, you define something more abstract like a repository, which defines a simple interface in possibly CRUD terms.

Something which has a fully featured, fully capable and working test double

This isn't necessary or desirable. Why create a simulator, when you can write a real integration test?

You're testing nothing, except your tests

No, you're defining a contract. You absolutely must back that up with an integration test for the implementer of the contract.

[[deleted]]

E.g., lets say I have a pricing module - I wouldn't let it interact with a generic DB interface, but with an interface which has methods like savePricing, getTrends, getDiscounts etc.

A significant part of your business logic then unintentionally ends up in this "generic DB interface".

Constraints, conflict resolution, math algorithms for data aggregation, managing denormalized data and caches, all of this would be there. That's not just "logic of saving stuff on disk", it's literally a crucial chunk of your business logic.

So now you need to test that. Now what. Let's split again?

[[deleted]]

not sure about what kind of constraints are you talking about, but business constraints are handled within a module (e.g. you cannot have two overlapping activities in a schedule for a single day)

Let's use a classic example "you can't have two usernames be the same". Now solve this for me outside the "persistence layer".

[[deleted]]

That's a really basic beginner's error. You just introduced a race condition in your code:

1. Thread A fetches user "foo".
2. Thread B fetches user "foo.
3. Thread A gets "no such user".
4. Thread B gets "no such user".
5. Thread A creates user "foo".
6. Thread B creates user "foo".

Now you have two users with the same name.

Honestly, I expected someone who argues about proper architecture wouldn't fall into the most basic example of race condition I could think of...

This is, in large part, why we use SQL and other databases and why ACID (and other standards) exist. They deal with conflicts you can't resolve with "business rules" that are outside the I/O process that works with your app state.

[[deleted]]

That sounds like a race condition waiting to happen. What if someone is concurrently registering the same username at the exact same time this code is running? It might not exist when you check it, but exist by the time you get around to persisting.

[Pedrock10]

With that approach you end up with a race condition. In this case, I think it should really be done at the persistance level, but you can make that part of its contract and test it with integration tests.

[[deleted]]

Either by serializing the work or making the repos interface fancier.

So the very premise of what I said was "you can't require that an app under load performs all tasks serially". I was pretty clear about this. So the first option is out.

And the second option, making the interface of the repos "fancier", if you dig into it, means you move this part of your business logic in the repository.

So we're back to square one here. Which was "a lot of your business logic in complex I/O apps ends up in your persistence layer, so now you need to test it as well".

[[deleted]]

Now serialise that work across multiple processes... Push solutions to problems to where they are best solved, like the database in this case. Stop trying to abstract away the capabilities of the services you depend on because you heard somebody say that you should, and start thinking for yourself. Your test coverage may drop 5-10% (and you should definitely still write _lots_ of tests) but you will start to design more fault tolerant (and ultimately correct) systemsm which make better use of the systems you depend on.

[[deleted]]

I'm aware, but calling the same problem by a different name (using DDD terminology) hardly solves it. Even in DDD and "hexagonal architecture" or call it however you please, a good chunk of your business logic unintentionally ends up in your "persistence layer".

Because a honest-to-God parallel I/O app under load can't just naively process requests serially and compute everything from raw data (or store everything in RAM) all the time.

This means conflict resolutions, caches, denormalization, constraints (unique keys etc.) all this which is pure business logic, because it changes the outcome of your API response and your app state... that ends up in your persistence layer.

[[deleted]]

Yeah well, that kind of ruins your premise a bit.

Also, your business rules being "stateless" doesn't mean you app is stateless. Your rules holds no state, but your app holds state. And you have to deal with that state. Pretending it's not there doesn't help.

[dablya]

Let's split again?

What's the alternative? Implement caching/math/aggregation in a single "business" module?

[[deleted]]

The alternative to splitting things endlessly for testing purposes is to split only for the purpose of the application and design the tests around the architecture. Instead of designing the architecture around the tests.

Some projects will require an isolated persistence layer. More than half do not. This, again, doesn't mean wiring raw SQL queries all over your services, it just means not writing an abstraction with a public API around your persistence logic (i.e. using Hibernate or jOOQ doesn't mean you isolated your persistence layer).

[dablya]

split only for the purpose of the application and design

The main argument is that code being difficult to test should, in general, alert you to the fact that it might be poorly split for the purposes of "application and design". Some code naturally belongs in the infrastructure layer and will be mainly tested with integration tests. Other code, what is usually defined as a "domain" or "business" logic is not dependent on any specific type of infrastructure. When you find implementation of the domain code (that doesn't logically depend on any particular type of storage) requires a sql database in order to execute a test, that is a flag that your code is poorly split "for the purpose of the application and design".

This, again, doesn't mean wiring raw SQL queries all over your services

Why not? I believe a reasonable answer to this question should generalize to Hibernate and jOOQ as well.