Reputation: 89
I read that generally private methods has no need to unit testing because they are tested by public methods. So what should I do in service layer?
I have a service layer and a method to add an article to database. This method checks some logics about article and its files then save it via repository:
public void AddArticle(ArticleDto article)
{
CheckArticleFiles(article);
CheckArticleTitle(article.Title);
CheckArticleSummary(article.Summary);
CheckArticleBody(article.Body);
_articleRepository.AddArticle(article.MapToDbModel());
_articleRepository.SaveChanges();
}
All of the checking codes are public now to enable them for testing which I think it's not the correct way(because of OOP principles). And on the other hand if I change them to private they should be tested by AddArticle method. In this case I think that it's not a unit testing. because it is not a unit and this method act as an orchestrator.
This situation is happen if I use domain model which has its private logics. And the logics are the most important part need to test which no need to be public and even protected
So what are my mistakes here?
Upvotes: 0
Views: 205
Reputation: 5939
Often parts of interfaces are designed to be used only in certain contexts (during configuration only, during initialization only, from certain threads only, from test code only, in certain system states only, ...). The possibilities of programming languages to express this such that real technical barriers are created is fairly limited (typically you have public and private, often protected, sometimes a bit more) - and often enough there exist tricks to overcome barriers as public or private declarations, for example in C++ using preprocessor tricks, in Java and similar languages using introspection. In Python, for example, such mechanisms are completely replaced by conventions - if you don't follow them, sooner or later you loose.
Looking at things from a perspective that does not focus on a particular programming language I thus prefer to consider public
, private
etc. more as logical concepts representing architectural constraints. For example, extracting some private functions that represent implementation details into a separate class as public methods does not change their nature of being implementation details: That other class then should certainly not be used by other components as well, and test code that uses that class is not any less affected if these implementation details change. I mention this only to demonstrate that I see not much of a difference in making a method public but telling others not to use the method compared to extracting it to a different class and telling others not to use that class.
In your case, you would like to express that certain functions shall be considered as implementation details (private
) but shall still be accessible for unit-testing purposes. And, yes, I agree that it can make more sense to test the private
functions directly than testing them indirectly from what you call the service layer. It is certainly a tradeoff, but from my experience I also know such cases.
The simplest approach is (if the language you use does not offer any better tricks like C++'s friend
construct), as you have suggested, to make the methods public and document they are not intended to be used. If you want to make this more prominent, one possibility is to make a function publicly accessible but express the limitations in usage by naming. A function foo
that shall only be used by tests could be named forTestingOnly_foo
. Since that makes it ugly also when the function is called in a private context, foo
could still be a private function with a non-private wrapper forTestingOnly_foo
.
If you can establish a naming convention for such scenarios, you could create a simple checker that verifies that nobody (except from a testing context) accesses any of these 'publicly accessible but logically private' methods, and use it in the build to make it fail if a misuse is detected. Whether such approaches are needed depends a lot on the organisational culture you are working in.
Upvotes: 1
Reputation: 6335
I would have unit tests around each piece of business logic.
CheckArticleFiles(article);
CheckArticleTitle(article.Title);
CheckArticleSummary(article.Summary);
CheckArticleBody(article.Body);
you're not showing any code but it seems that these methods have certain business rules and that's what you need to test.
You also want to make sure that your repo actually works, so you could have a few integration tests around that, here is some data, I expect the database data to look like this ...
One note, an Add method should return something so you can at least do some sanity checks, for example return the unique id of the created article and this way you know if your method actually worked or not, plus it allows you to do other interesting things, like building a link to the newly created article, because you now know the id.
It's very difficult to add more seeing as you haven't actually shown any useful code so far. For example, I would have an Article class where all the business logic would go. The repo and the saving part would be outside of that. Having a public method which checks the title, is not a bad thing at all, it allows you to check things individually rather then checking the whole thing in one. This depends on how you use it as well.
Upvotes: 1