Add upgrade path tests for contact category config conversion

I discussed this to some length with @alexpott already, and I'd like to see "combined/cumulative" ConfigUpgradePathTest; i.e.:

1. A full blown D7 site with each and everything that can be configured (in variables + Configurables).
2. The dump is loaded once. D7 variables + Configurable tables are adjusted once.
3. The entire thing is upgraded once.
4. ConfigUpgradePathTest will potentially get large, with a plethora of assertions for the expected upgraded config.
5. The test class should provide helper methods to simplify assertions.
6. Even better, screw custom assertions methods — create a ./upgraded_config/ directory with actual config files and simply compare and assert that the final configuration after upgrading is identical to that directory.

Yep we can't use entity_create() in a hook_update_N.

#3 looks mostly good - I've rerolled and combined the tests so we don't have to run the upgrade twice. I've also added the langcode into the configuration entity as at the moment this is added to all entities.

Hmmm... and we've also got to create a manifest file :(

Patch adds a generalised method for hook_update_N functions to create a manifest file, uses during contact category entity creation and tests it.

Great - existing "X to config entities" updates did not create manifest entries so far, and currently the one in #1852966: Rework entity display settings around EntityDisplay config entity doesn't either, so this is badly needed.

Minor nitpick :
- The function is not really about creating a manifest file, but inserting entries in it (creating the file if needed)
- We have similar "generic helper functions for special use in hook_update_N() where you're not supposed to use regular APIs" in core right now, and they're named _[system]_[7000|8000]_action()
_update_8000_field_sql_storage_write()
_update_7000_field_create_field()
_update_7000_node_get_types()

--> may I suggest _entity_8000_manifest_insert() ?

Side note: in #1802750-5: [Meta] Convert configurable data to ConfigEntity system I wondered whether there would be a way to simplify creation of UUIDs too. Like manifest files, it's really easy to forget (the upgrade for image styles still misses UUIDs), and you get a fully functional system that will only break when you try to import config.
Maybe we could discuss this over there ?

Also: it seems the function currently supports re-adding the same entry even if it already exists in the manifest ?
That is going to be very handy, because you don't really need to care whether the config file exists or is being created.
So IMO this behavior should be advertised as a feature in the phpdoc :-)

right now, and they're named _[system]_[7000|8000]_action()
_update_8000_field_sql_storage_write()
_update_7000_field_create_field()
_update_7000_node_get_types()

--> may I suggest _entity_8000_manifest_insert() ?

Your examples and then pattern doesn't seem to match? Shouldn't it be _update_8000_entity_manifest_insert()? And not sure about entity vs. config, the manifest file is arguably more related to config? It is implemented in the entity storage controller, but the entity system doesn't know/care about it.

Tests and update function looks great otherwise.

@Berdir: er, yes, sorry, #13 is pretty mixed up :-/.
Right, something like _update_8000_config_manifest_insert() would be good.

Well, I was about to roll a patch to rename the function, but I bumped into the helpers that currently exist in this "variable / config / state" area:
update_variable_get|set|del() (operate on {variables}, probably bound to disappear at some point)
update_variables_to_config()
update_variables_to_state()
update_add_uuids()
The update_config_manifest_create() function added in this patch does fit consistently there.

So it seems we do have different patterns :
- _update_[N]000_*() for update-safe versions of API functions
- update_*() for general helpers
Not sure this is ideal, given that the separation is not really clear cut, and that the latter functions then sit next to internal update mechanisms like update_build_dependency_graph(), update_is_missing()...
The conceptual difference between "helper funcs for update hooks" & "internal update.php mechanisms" is much more relevant IMO,

But at least the function as it is named right now is somewhat consistent with the current update_variables_to_config() & friends, so there's no reason to block this specific patch...

Just slightly renamed the function and adjusted the phpdoc - it's not only used to create manifest files, but also to add entries in an already existing manifest.
Don't credit me.

Opened #1871270: Standards for hook_update_N helpers for the naming discussion.

#18: 1817182-contact-upgrade-18.patch queued for re-testing.

Committed and pushed to 8.x. Thanks!

Ouch, actually the newly added update_config_manifest_add() helper doesn't work if the $ids of the config entities contain dots.
$config->set($name, $value) treats dots in $name as nesting indications, i.e $config->set('my.id', $value) results in $config['my']['id'] = $value.

ConfigStorageController::save() saves manifests by doing :

$config = config('manifest.' . $this->entityInfo['config_prefix']);
$manifest = $config->get();
if (!in_array($this->getConfigPrefix() . $entity->id(), $manifest)) {
  $manifest[$entity->id()] = array(
    'name' => $this->getConfigPrefix() . $entity->id(),
  );
  $config->setData($manifest)->save();
}

(the if (!in_array()) check seems weird there, but the point is the data is written as a whole by setData())

Left a comment about the if (!in_array()) check in #1697256-152: Create a UI for importing new configuration.

Meanwhile, here's a fix for update_config_manifest_add().

Makes sense to me.

Committed to 8.x.