Problem/Motivation

As part of #3066512: Add checks for syntax and display of help topic Twig template files, we are adding a test that installs all the modules and themes, and then verifies that all help topics can be rendered. That test class is core/modules/help/tests/src/Functional/HelpTopicsSyntaxTest.php.

As part of #3037230: Finalize the merge of Help Topics into Help, we wanted to move the test of help topic rendering into \Drupal\Tests\system\Functional\Module\InstallUninstallTest, in the assertHelp() method

That method is currently verifying that modules and themes implementing hook_help (which we'll be removing/deprecating someday but now required to pass doc-gate), and it should instead verify that all of the help topics for that module can be viewed. We'll need additional tests to do the same for themes and install profiles, which cannot have hook_help but can have help topics. I haven't yet investigated where those should live.

There was intent to unify discovery of enabled modules but core now using module_list

Proposed resolution

Just remove the TODO as it makes no sense to merge tests until fate of hook_help() in #3031642: Deprecate hook_help() and combine with Topics

Outdated suggestion was

I think we need to do the following:

a) As in the current HelpTopicsSyntaxTest, with all the core modules and themes installed, run the syntax checks on all the help topics that exist. This will ensure correct syntax of all of the topics, and also that all the Related topics links exist.

b) With a given module, its dependencies, and the core required modules being the only modules installed, verify that all of the module's topics can still be viewed (same for themes). This will make sure that we aren't relying on another module/theme somehow that isn't actually a dependency, such as trying to make a link to a route that doesn't exist unless a certain module is installed... This may not be necessary if #3090659: Make a way for help topics to generate links only if they work and are accessible is taken care of.

Remaining tasks

patch/review/commit

User interface changes

no

API changes

no

Data model changes

no

Release notes snippet

CommentFileSizeAuthor
#15 3074040-15.patch838 bytesandypost

Comments

jhodgdon created an issue. See original summary.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Issue summary: View changes

Adding some additional details of what I think we should test and how.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

smustgrave’s picture

smustgrave commented
Status: Active » Postponed (maintainer needs more info)

This appears to have already been done. If someone could confirm.

I'm no longer seeing a file HelpTopicsRenderTest.php and help testing appears to be in InstallUninstallTest

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Postponed (maintainer needs more info) » Active

Gonna check it as help topics are merged to help module

andypost’s picture

andypost
he/him
Primary language Russian
commented

There's still TODO in HelpTopicsSyntaxTest so the issue still actionable

andypost’s picture

andypost
he/him
Primary language Russian
commented
Issue tags: +Needs issue summary update

summary outdated, still makes sense to check supplied topics in common test which installs every module

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Active » Needs review
StatusFileSize
new 3074040-15.patch838 bytes

There's \Drupal\Tests\system\Functional\Module\InstallUninstallTest::assertHelpwhich is checking required hook_help() implementation for core modules.

I think we should leave the test as is because
- it's purpose to do syntax checks of topics, which is unrelated to system module
- the test already self explaining but InstallUninstallTest doc-block is totally unclear and doing more then decumented

Install/uninstall core module and confirm table creation/deletion.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

For the IS update.

Change you made looks simple enough.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Needs issue summary update
Related issues: +#3031642: Deprecate hook_help() and combine with Topics

Trying to run tests locally

Testing Drupal\Tests\help\Functional\HelpTopicsSyntaxTest
Test 'Drupal\Tests\help\Functional\HelpTopicsSyntaxTest::testHelpTopics' started
Test 'Drupal\Tests\help\Functional\HelpTopicsSyntaxTest::testHelpTopics' ended


Time: 06:11.269, Memory: 4.00 MB

OK (1 test, 13382 assertions)
...
real	6m12,258s
user	0m0,040s
sys	0m0,022s

Second one can't complete (fails installing mysql as I'm using sqlite)

Testing Drupal\Tests\system\Functional\Module\InstallUninstallTest
Test 'Drupal\Tests\system\Functional\Module\InstallUninstallTest::testInstallUninstall' started
Test 'Drupal\Tests\system\Functional\Module\InstallUninstallTest::testInstallUninstall' ended


Time: 12:53.326, Memory: 4.00 MB

There was 1 error:

1) Drupal\Tests\system\Functional\Module\InstallUninstallTest::testInstallUninstall
Behat\Mink\Exception\ExpectationException: Current response status code is 404, but 200 expected.

...
real	12m55,073s
user	0m0,038s
sys	0m0,045s
smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Don't mind marking this for you.

quietone’s picture

quietone commented
Status: Reviewed & tested by the community » Needs work

I looked at the patch and see that this is not moving a test. Setting back to needs work for a title update.

On the other hand the comment being removed states

so that it will test with only one module at a time installed and not duplicate the effort

As I read that it means that the intention was to test each module without other modules installed.

Looking at the history I find support of that interpretation.

The @todo comment was added in #3066512-15: Add checks for syntax and display of help topic Twig template files. The following comment in the issue states,

c) Write a test that would install all the non-hidden and non-test modules at once, and verify all the help topics can be rendered. This would not be as rigorous as testing with the modules individually installed, because it might mask problems in routes not being defined, but it would be better than nothing.

It does seem that this is to add more testing. Is there a reason not to add this level of testing?

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs work » Postponed
Parent issue: #3037230: Finalize the merge of Help Topics into Help » #3031642: Deprecate hook_help() and combine with Topics
Related issues: -#3031642: Deprecate hook_help() and combine with Topics +#3037230: Finalize the merge of Help Topics into Help

Proper parent is #3031642: Deprecate hook_help() and combine with Topics

when we consider to require help topics instead of hook_help() this action becomes doable

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Postponed » Active
andypost’s picture

andypost
he/him
Primary language Russian
commented
Related issues: +#2975163: InstallUninstallTest is taking 20 minutes to run
andypost’s picture

andypost
he/him
Primary language Russian
commented
Title: Move testing of help topic rendering into InstallUninstallTest » Move testing of help topic rendering into child of GenericModuleTestBase

There's GenericModuleTestBase now

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.