Get tests passing

[rosiel]

GitHub Issue: #954 #887

• Other Relevant Links (Google Groups discussion, related pull requests,
  Release pull requests, etc.)

What does this Pull Request do?

Attempt to get more tests running.

What's new?

• removed the dependencies clause in the installed breadcrumbs setting; it was complaining that it was missing a schema for it, but it shouldn't have been there in the first place.
• Adds ("injects") FileUrlGenerator into our image field formatter
• Attempts to "fix" tests that were breaking looking for field_tags on a media add form, where the correct name seems to be field_media_use

• Does this change add any new dependencies? no!
• Does this change require any other modifications to be made to the repository
  (i.e. Regeneration activity, etc.)? no!
• Could this change impact execution of existing code? hopefully fixes the tests. otherwise no.

How should this be tested?

CI runs the tests... how green are they?

Documentation Status

• Does this change existing behaviour that's currently documented?
• Does this change require new pages or sections of documentation?
• Who does this need to be documented for?
• Associated documentation pull request(s): ___ or documentation issue ___

Additional Notes:

Any additional information that you think would be helpful when reviewing this
PR.

Interested parties

[rosiel]

I figured out why it couldn't write to field_media_use ("Tags") (formerly field_tags) - the config for the form referred to the old field_tags so the field_media_use wasn't showing up.

But that doesn't solve the problem that the test passes if the tag/use is left out entirely. This shouldn't be happening, because the derivative action config was set to use the Preservation Master term. 🤯

[rosiel]

Regarding commit dd514a3, the checks are there firstly to remove the deprecation notices. They're all FALSE because it seemed easiest, but we could change them to ->accessCheck(TRUE) if that's the prevailing opinion. I'd like some advice on this.

[jordandukart]

The former option prior to the change record was to set accessCheck to TRUE if it wasn't explicitly set so your change as it stands as of 4eae636 should be good!

[rosiel]

Thanks Jordan. Seth and I discussed it on Slack and he suggested leaving it at the default behaviour (but potentially opening up our code to allow a parameter telling it not to check access).

I'd appreciate it if anyone can help with this last (hopefully!) test. It's failing now on php 8.1, here:

Testing 
..............E...........                                        26 / 26 (100%)

Time: 26:42.239, Memory: 18.00 MB

There was 1 error:

1) Drupal\Tests\islandora\Functional\JsonldTypeAlterReactionTest::testMappingReaction
Exception: Deprecated function: mb_strtolower(): Passing null to parameter #1 ($string) of type string is deprecated
Drupal\Core\Config\Entity\Query\Condition->compile()() (Line: 39)


/opt/drupal/web/core/lib/Drupal/Core/Test/HttpClientMiddleware/TestHttpClientMiddleware.php:49
/opt/drupal/vendor/guzzlehttp/promises/src/Promise.php:204
/opt/drupal/vendor/guzzlehttp/promises/src/Promise.php:153
/opt/drupal/vendor/guzzlehttp/promises/src/TaskQueue.php:48
/opt/drupal/vendor/guzzlehttp/promises/src/Promise.php:248
/opt/drupal/vendor/guzzlehttp/promises/src/Promise.php:224
/opt/drupal/vendor/guzzlehttp/promises/src/Promise.php:269
/opt/drupal/vendor/guzzlehttp/promises/src/Promise.php:226
/opt/drupal/vendor/guzzlehttp/promises/src/Promise.php:62
/opt/drupal/vendor/guzzlehttp/guzzle/src/Client.php:182
/opt/drupal/web/core/tests/Drupal/Tests/DrupalTestBrowser.php:137
/opt/drupal/vendor/symfony/browser-kit/Client.php:404
/opt/drupal/vendor/symfony/browser-kit/Client.php:324
/opt/drupal/vendor/friends-of-behat/mink-browserkit-driver/src/BrowserKitDriver.php:719
/opt/drupal/vendor/friends-of-behat/mink-browserkit-driver/src/BrowserKitDriver.php:494
/opt/drupal/vendor/behat/mink/src/Element/NodeElement.php:153
/opt/drupal/vendor/behat/mink/src/Element/NodeElement.php:161
/opt/drupal/web/core/tests/Drupal/Tests/UiHelperTrait.php:100
/home/runner/work/islandora/islandora/build_dir/tests/src/Functional/JsonldTypeAlterReactionTest.php:33
/opt/drupal/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

I tried my darndest with a breakpoint at the various implentations of mb_strtolower(), and clicked through it hundreds of times, but the only times i found "NULL" imply the problem is:

• during the creation of the new field (line 33 of JsonldTypeAlterReactionTest.php)
• during a chain of preSave() functions
• it tries to update/create the entity_view_display, and is checking if one exists with the desired UUID - problem is, the UUID it's checking for is NULL. The UUID gets passed in to mb_strtolower() when looking for matching configs, so I think that's where it's failing. It might also fail the same way for entity_form_display.
• But I can't figure out why!

[jordandukart]

Looks like it might be related to https://www.drupal.org/project/drupal/issues/3302838 @rosiel.

EDIT: Though sounds like it's more of a result of something else being flubbed, hmm.

[rosiel]

It turned out to be that the configs we loaded from .yml files during the IslandoraFunctionalTestBase:setUp() function were missing UUIDs (and they weren't being generated from the way we copy the YML). When adding a field, it tried to update one of those (an entity view display or form display) but the lack of UUID was causing it to fail.

[jordandukart]

@rosiel found and addressed 5+ year old bug in the test fixtures to get these to run so props to her, rest of what's here are deprecations.