Make Upgrade Status tests Drupal 9 compatible

+++ b/tests/modules/upgrade_status_test_contrib_error/src/Controller/UpgradeStatusTestContribErrorController.php
@@ -12,7 +13,7 @@ class UpgradeStatusTestContribErrorController extends ControllerBase {
-      '#markup' => format_string('I am @deprecated', ['@deprecated' => 'deprecated']),
+      '#markup' => new FormattableMarkup('I am @deprecated', ['@deprecated' => 'deprecated']),

+++ b/tests/modules/upgrade_status_test_error/src/Controller/UpgradeStatusTestErrorController.php
@@ -2,12 +2,41 @@
-    menu_cache_clear_all();
+    $this->cacheBackend->invalidateAll();

+++ b/tests/modules/upgrade_status_test_error/src/DummyTestClass.php
@@ -2,14 +2,14 @@
-class DummyTestClass extends WebTestBase {
+class DummyTestClass extends BrowserTestBase {

These are explicitly included as deprecated API uses to test them. If we are fixing them then the tests will fail because we are not finding any deprecated APIs.

Steps for this issue, as explained in the proposed resolution is:

1. Either wait for the first deprecation in Drupal 9 and adapt tests for that or
2. Add our own stub API that we deprecate so we can identify that finding it works. Then we can be Drupal 9 compatible cleanly.

Since (1) did not happen and may not happen for a while AFAIK, (2) would be the best to do. Add some dummy API of our own that we deprecate and use and can test that we found the use of said deprecated API. That would make our testing Drupal major version agnostic. Unfortunately fixing the use of deprecated APIs while we are still testing to find them even though they are not there does not move us forward.

Also the project page (https://www.drupal.org/project/upgrade_status) has a link to #3100361: Remove usage of file_directory_temp() if its not available for Drupal 9 compatibility where the file_directory_temp() part of the patch would belong.

Finally the REQUEST_TIME fix looks like is in order regardless of major version compatibility, please open a new issue about that with that part of the patch, thanks a lot.

@lauriii also reported #3123824: Uncaught Drupal\Core\Extension\InfoParserException on Drupal 9

Ok I made the module Drupal 9 compatible in #3133544: Make Upgrade Status work on Drupal 9 (except tests), but tests are still not. So the module could theoretically be run on Drupal 9 but we cannot yet prove with tests, since our tests are heavily dependent on the availability of the Drupal 9 deprecated APIs in PHP, twig, etc. So this remains standing for full compatibility.

That said, there is no reason to run the module on Drupal 9 yet, so I am not very rushed to fix this ASAP. We need to have a plan for each area being tested how can we have version agnostic tests.

Yeah I already committed the time property and use in #3133544: Make Upgrade Status work on Drupal 9 (except tests) and elsewhere based on @fgm's patch. I should have credited @rpayanm if I would have collated the issues. Sorry for that. The tests still need fixing, but not the way proposed here :) If we fix the deprecated API use we are testing for, then our tests will not find them of course.

So moving to active, since this is back to the drawing board.

@ressa: if the module would not report errors in its test code, then how would we test that it finds errors? Without errors?

@ressa: do you have a patch proposal? We'd need to exclude it for the sake of runtime display then and not for the sake of testing.

@ressa: also that would still not make the tests themselves Drupal 9 compatible, so would not resolve the goal of this issue and therefore should be its own issue to have cleaner reporting.

Made some exploration to the extent of this. We are testing five things that are outside the bounds of this project:

Let's see how does this patch fairs for now. I added a @todo for the ones that need to be revisited. In particular 3 and 4 need some solution to make the module compatible with Drupal 9.

The fails were due to two things (a) we categorize errors by source version number for severity and using X.Y.0 did not fit that so did not pass that test, using a concrete version number for now (b) error line numbers of course changed where I added additional comments to file.

This passes locally. Still need to figure out what to do about the remaining 8 specific @todo items.

Read up on the twig docs, and it is apparently possible to define our own deprecated filter to test that instead of raw. https://twig.symfony.com/doc/1.x/advanced.html#deprecated-filters -- would be great to get a helping hand in this. :)

The raw tag deprecation is very hard coded in src/Lexer.php:

    protected function lexRawData($tag)
    {
        if ('raw' === $tag) {
            @trigger_error(sprintf('Twig Tag "raw" is deprecated since version 1.21. Use "verbatim" instead in %s at line %d.', $this->filename, $this->lineno), E_USER_DEPRECATED);
        }
    }

I'll look into adding a custom twig filter though that we can check then. Fingers crossed.

Ok went by the samples at https://symfony.com/doc/current/templating/twig_extension.html -- this is not yet passing, but posting as a work in progress. Kept the raw usage for now until this passes.

Woah, woah. Turns out the fail was due to the twig error regexp not matching line numbers other than 0-9 :D. That's not good, alright.

Now removed uses of the raw tag, so we can be dependent of Drupal 8 / Twig 1 there too. Only WebTestBase left I think.

Aand now making it independent of WebTestBase also by defining our own deprecated base class. Wohoo!

Let's allow to work on Drupal 9 in the info file so we can actually test with Drupal 9.

Duh that puts the module into the "relax" category.

Should update composer.json too so composer can install it.

Meh, stray comma.

We can remove the "core: 8.x" from test info files on 8 without affecting how it is tested. This will at least uncover the next layer of fails on Drupal 9.

My limited grep did not find this 7.x core compatibility. It still does not affect D8 testing, let's see D9.

So due to the added Drupal compatibility info for test modules, this will get categorized as relax. Wondering why the others did not come up first... Hm.

+++ b/tests/modules/upgrade_status_test_contrib_no_error/upgrade_status_test_contrib_no_error.info.yml
--- /dev/null
+++ b/tests/modules/upgrade_status_test_deprecated_function/upgrade_status_test_deprecated_function.module

Not convinced that this needs to be its own module, it looks way too granular. Eg. the twig test modules both define and use their deprecated things. We can also just move the definition of this to the "contrib" "error" modules as an alternative (and if we want to super correct add a dependency to the "custom" module). The test deprecated base class is also in the error module.

Otherwise looks great, thanks!

There we go.

Actually I noticed one more accidental @andypost change. We intentionally test the 9.1 compatibility on one of the test modules. I don't think it would be a problem that 9.0 sites cannot run that test. Sites that run tests for deployment would usually not be outdated on 9.0 at this point.

All right, committed this!

Also credited @rpayanm for the $time replacement, because they were not credited for that elsewhere. (Even though that did not happen in this issue after all).

I opened #3201156: Make Upgrade Status logic make sense on Drupal 9 to continue with Drupal 9 to Drupal 10 support.