Comment pager breaks the render cache

The one concern I have is that if people change the "Comments per page" setting, then the cache wouldn't be cleared. There are two potential solutions:

1. Embed the number of comments per page in the cache key.
2. Employ cache tags to ensure that when the "comments per page" setting is changed (for a given comment field instance — not everywhere!), the corresponding cache entries are deleted.

Option 1 seems the simplest and most efficient solution (per-comment field instance cache tags… that doesn't sound very efficient), so that's what I went with.
I ran into two problems when retrieving the per_page comment field instance setting:

1. Getting the commented entity's bundle requires entity loading (see the first @todo I added)!
2. Getting the commented entity's comment field name requires string parsing (see the second @todo I added).

Both could be solved by including the necessary data in the comment entity as additional properties. Are there reasons to not do that? Then CommentViewBuilder::getBuildDefaults() would become simple again.
So the current patch needs refinement, but I believe the logical changes are sound.

Note that this also means that we must adjust the cache keys even if there isn't any ?page=X URL query, because if the number of comments per page is changed, then "page zero" (for which there would not be any comment pager-related identifier in the cache keys/ID) won't be updated.
(This also shows how Berdir's #5 remark was very important — we could've ended up with two numeric keys just for comments, yet other modules might contribute to it too! Any externally added values to cache keys should be uniquely identified, otherwise they may be misinterpreted.)

Finally: is test coverage a necessity or a nice-to-have?

I don't see how this is overengineering? This is about correctness. The code seems overengineered for sure, but that's because most necessary metadata is missing.

You're right though that what I proposed still misses to react to other comment settings changes :/ And that's when I realized I've been incredibly DUMB for not seeing that the $entity_type . '_view' cache tag is already taking care of all this :/ I'm just overcomplicating things. I guess that's what you meant, then?

So #6 was indeed already all we needed to add. Except that it's indeed appending its cache key to the wrong entity. I'm working on fixing that.

The solution is simple and elegant :)

Multiple comment field pagers hell :)

Sadly, it's complicated by the fact that #731724: Convert comment settings into a field to make them work with CMI and non-node entities made it possible to have multiple comment fields per entity type… yet it failed to update the pager to ensure each comment then has its unique pager. That may seem like an exotic edge case, but without unique pagers, we cannot update the render cache key correctly either! Sadly, that in turn seems to require pretty extensive changes to all of Drupal core's pager logic contained in pager.inc, which only allows one to assign a numeric identifier to a pager, to allow for multiple pagers on a page. But that's problematic too, because it makes it impossible (AFAICT) for us to figure out which pager belongs to which comments reliably.
So, the current sad reality is that the one ?page=X query will control all comment pagers simultaneously. And if one set of comments requires e.g. 2 pages and the other requires 5 pages… then you'll only be able to view 2 pages of either set of comments!
In other words: pager handling in general looks like it needs to be refactored. But that's obviously out of scope.
So, for dealing with all that, I opened a new issue at #2155387: Multiple comment fields per entity are supported, but break horribly when they need pagers.

Should be green now.

But but but … it was a legitimate failure and I fixed it locally! :(

Thanks for the advice, Berdir, I'll fix that :)

In this reroll:

1. Addressed Berdir's feedback in #16.
2. Now addressed the whole pager problem because larowlan is awesome: he fixed the whole pager problem last night already: #2155387: Multiple comment fields per entity are supported, but break horribly when they need pagers. larowlan++.
   I'm postponing this patch on that patch, though.
3. In addressing that, I temporarily already enabled node/comment render caching, this caused me to find that when you update comment settings (e.g. enable/disable comment threading), render cached comments aren't being deleted (and CommentPagerTest detected that problem). A simple fix ensures that CommentViewBuilder::resetCache() gets called, which fixes the problem.
   (That change could either be part of this patch or the patch in #2151459: Enable node render caching, but I think we might as well fix it here.)
4. #2031725: Move all entity display interfaces to the core component is the reason for the failed test, so rerolled against HEAD.

Good point. I can move that condition even higher though! :)

Interdiff against #21.

Thanks!