Mark test groups as belonging to modules in UI

Sorry, that's not the right fix... the different capitalization exists by design:

The capitalized group names are referring to core component names. The lower_underscored group names are module names. See:

- #2057905-47: [policy, no patch] Discuss the standards for phpunit based tests
- #2297541: [policy, no patch] Secondary/additional test @group names

The groups are intentionally different, because we want to be able to selectively run either tests from a component or a module.

We probably want to change the sorting algorithm, so that all group names starting with an uppercase letter appear before lowercase group names.

Additionally: #2296615: Accurately support multiple @groups per test class

However, that UI change might take a bit longer, so it probably makes sense to do a quick stop-gap fix for the sort function here.

Lastly, a few of the duplicate group names are bogus. They're caused by System module's subdirectories in Tests, which are supposed to map to Drupal core components.

The duplicate action/Action group is caused by the test in system.module's "Action" subdirectory. That seems to be misplaced there, a left-over from times when actions were part of system.module. — After moving the test to action.module, the group can be changed to "action", because that is the originating module.

Likewise, the Block group is caused by Tests\Block\SystemMenuBlockTest. There is no Block component in Drupal core. The test appears to be an integration test for System module's menu block itself, so it doesn't belong into the Menu subdirectory. We can move it into the Tests\System subdirectory or the Tests directory itself.

The other groups should be correct, AFAICS.

I'll try to do a patch for the above points.

Double-checking that Action test once more, it indeed appears to test the Action plugin interface of the Drupal\Core\Action component, so I was mistaken there.

1. Sort groups case-sensitively to make capitalized core components groups appear first.
2. Moved SystemMenuBlockTest into System subdirectory.
3. Fixed group and docs of SystemListingCompatibleTest.

Yeah, that specific "Block" case is a special/bad case, which is corrected by the patch.

But otherwise, I think we all have the same expectations:

1. To run the tests of the Routing core component, you run the "Routing" group. (component tests)

2. If there was a routing.module in contrib, then you run its tests by running the "routing" group. (module tests)

That differentiation always existed. The only difference to the past is that all group names were custom/made-up human-readable strings, and test groups of components typically used an " API" suffix in their name (e.g., "File API"), whereas test groups of modules typically used the human-readable module name (e.g., "File").

The new test groups are the same as before, but they are using the literal internal name of the originating module or core component now. So tests for the File core component are using the literal "File" component name as group name. And tests of file module are using the literal "file" module name as group name.

The direct mapping simplifies to find and use a group in the UI and especially the CLI. Previously, you first had to figure out and look up what the group name of tests to run is. Now you immediately know what group you want to run, because all groups are identical to the internal names of modules and components.

In terms of the specified @group names in code, I think that makes a lot of sense, no?

Of course, the presentation of the list of tests in the web UI could probably use some improvements. Although with regard to the web UI, I think we should directly go ahead and change it to support multiple groups (instead of singular); i.e., #2296615: Accurately support multiple @groups per test class

Coming here since I noticed #697760: Replace getInfo() in tests with native phpDoc + annotations (following PHPUnit) changed
1. all @group Locale were changed to locale
2. removing the name from getInfo seems to have removed "Translation" from a bunch of locale tests (so someone looking/searching in the UI for translation will not find locale tests anymore)
3. some of the descriptions were long, and now the one line summaries are... longer than 80 chars,
4. LocaleTranslateStringTourTest changed from Tour to locale. (Guess #2296615: Accurately support multiple @groups per test class might be the place to deal with Tour).

Re-roll of #3.

Have a look at https://www.drupal.org/documentation/git/configure on how to configure git so that it reports that file move as a move and not removing/adding a new file, which is very hard to review.

Replace removing/adding of the patch in comment #10, with rename.
Please Review

Thanks, patch looks better.

This might be a viable option, but having a look at a few example groups, there are still things that are off. For example, we have tests with multiple groups that in the current UI, doesn't really make sense. See screenshot, note that I have a lot of contrib modules here, that sometimes don't follow the standard of @group == module/project name.

- The first one in the list is now the group "Access", with only a single test. that also has filter group, we should make sure that group is first, so it uses that in the UI.
- Then we have Action, also only just one, not sure what to do with that, probably keep, the problem is that this is the action API test which lives in system.module/core, not action module (the UI)
- TextFormatElementFormTest has just group Form, I think that should be in filter, as it is a filter.module test.
- in Menu there are two menu_link_content tests, based on this, they should be group menu_link_content?
- One test in Shortcut, that should be shortcut
- one Views test that should be views
- condition_test is strange, why do we have a test in a test module? Should that be in system and/or group Condition?
- configuration system tests are currently all in config group (they are also actually in that module), even though most of them are API tests of the core component. Not sure what to do there, probably keep it for now, changing something there would be a bigger issue.

I will fix some of the contrib modules in the screenshot.

I think if this is how we want to do it, then we should either update or create a new change record and also document somewhere else how @group is meant to be used:
- lowercase for modules/projects, uppercase for components
- if multiple groups, then the first should be the one it is supposed to be shown in the current UI

#2422019: Don't use reflection for parsing test annotations has landed. So lets do a retest.

As commented in #2427191: Test-runner ignores @group annotations for PHPUnit-based tests in modules., there was so far no attempt to solve that problem here. We can, of course, especially since AFAIK alexpott doesn't like the approach this patch took so far.

I'm not sure we should get rid of the separation between a core component (like Action) and a module (like action). Modules/projects are currently grouped with their lower case machine name, at least that's what core and 80% of contrib is doing.

That has advantages, for example, it allows to relatively easy get from the project name to the test group, which I am using for http://d8ms.worldempire.ch/, see https://github.com/Berdir/d8modulestatus/blob/master/build.sh (line 45 right now), with the mentioned overrides (around line 26) for the 20% that currently does not follow this standard. That said, I will probably switch to a find + using --file in the near future, but there are some bugs there that I'd like to fix first like #2383395: `run-tests.sh --file` disregards missing dependencies.

I'm also not sure what we'd use as a @group then instead, the project name? And how to differentiate between core components and modules?

This really is a huge usability issue for people using tests from the UI. There's not a single person at any sprint I've attended who's looked at this and intuitively got what the point of the separation was intending to do. Instead they just get a glazed confused look on their face.

I totally agree that its confusing but to be honest, most of the time this grouping is not useful anyway as you start with search for a test by name and that's it.

For modules I'd be fine with having "action module", "node module" and "block module". In case this would categorize more system level tests into the module category,
it shows a problem in the way how we have structured them already.

On top of that it feels like manually being able to specify the group is kinda something you maybe not know, but instead there should be some magic
which adds "action module" in case your test is part of a module (we need to special case system though), and otherwise somehow figure out the component name using some heuristic.

This adds ' module' to the end of module group names in the form. Whether a group comes from a module name is determined by capitalization convention, so anything that's the same after strtolower() is assumed to be a module.

Also injects test_discovery service to the form builder (removing some deprecated simpletest_test_*() calls.)

Related: #2477717: SimpleTest Master Page possesses Duplicate ID's in HTML

Also: I keep running into this while testing: #2476139-10: Fix UI test fail in SimpleTestBrowserTest Hopefully the testbot fares better.

Todo: Bump to 8.1.x.

Looks like this:

:-)

Patch applies locally on 8.1.x, so I'm bumping this to 8.1.x and re-running.

Still applies to 8.2.x, rerunning test.

Nice! Its always fascinating that people actually use the simpletest UI. There is almost no value in it compared to using --browser.

OH: @dawehner the UI a gateway to CLI tests. Like git tower/source tree is to CLI git.

Also the slowest loading page for rendering so it's also a good place to test rendering performance improvements.

Removed the test_discovery injection which is really out of scope here, and that will happen in this issue anyway: #2801817: Inject services into SimpletestTestForm and functional cleanup for the test_discovery service

This patch causes the form to do extension discovery, and then checks each group to see if it's also an extension name.

It then adds the extension type to the group name, so EG 'node' becomes 'node module.'

See the screenshot and notice 'standard profile.' :-)

+++ b/core/modules/simpletest/src/Form/SimpletestTestForm.php
@@ -135,16 +148,30 @@ public function buildForm(array $form, FormStateInterface $form_state) {
+    // Get a list of all available extension names.
+    $extension_discovery = new ExtensionDiscovery($this->appRoot);
+    $extension_names = [];
+    $extension_types = ['module', 'theme', 'profile', 'theme engine'];
+    foreach($extension_types as $type) {
+      $extension_names[$type] = array_keys($extension_discovery->scan($type, TRUE));
+    }

So this adds file systems scans, are we okay with that? I'm just wondering.

simpletest_test_get_all() does the same thing via TestDiscovery. One should cache the data for the other via ExtensionDiscovery's static cache.

Reroll of #29.

1. +++ b/core/modules/simpletest/src/Form/SimpletestTestForm.php
   @@ -147,16 +160,30 @@ public function buildForm(array $form, FormStateInterface $form_state) {
   +        if (in_array($group, $extension_names[$type])) {
   

   I'd also use TRUE as third argument when dealing with strings

2. +++ b/core/modules/simpletest/src/Form/SimpletestTestForm.php
   @@ -147,16 +160,30 @@ public function buildForm(array $form, FormStateInterface $form_state) {
   -      $group_class = 'module-' . strtolower(trim(preg_replace("/[^\w\d]/", "-", $group)));
   +      $group_class = 'group-' . strtolower(trim(preg_replace("/[^\w\d]/", "-", $group)));
   

   Works for me, that's certainly the right change.

It looks like I managed to undo #2801817: Inject services into SimpletestTestForm and functional cleanup for the test_discovery service in the reroll, so it re-appears in the interdiff...

And changed the one line from #33. :-)

A quick re-roll due to a coding standards change.
#2776975: March 3, 2017: Convert core to array syntax coding standards for Drupal 8.3.x RC phase

Thanks for the re-roll.

+++ b/core/modules/simpletest/src/Form/SimpletestTestForm.php
@@ -61,24 +74,24 @@ public function getFormId() {
-    $form['actions'] = ['#type' => 'actions'];
-    $form['actions']['submit'] = [
+    $form['actions'] = array('#type' => 'actions');
+    $form['actions']['submit'] = array(

Isn't that backwards from our array declaration standard?

Note that work continues on #2296615: Accurately support multiple @groups per test class

Horrible re-roll. Try that again.

@Mile23 Can we just merge the duplicates? Instead of appending ' module' to the duplicates, which seems to be the visual difference. I'm hoping to make this page a bit shorter if possible.

They're different groups. For instance 'Action' refers to tests in the \Drupal\Core\Action namespace, and 'action' refers to tests for the action module.

I'd think I'd RTBC it if we could find a better way to organize these, maybe group them together as "Module" tests and "API" tests or something instead of just appending the type to it.

We're going to do some more shuffling around in #2858652: Support multiple @group test annotations in Simpletest UI and #2021077: make search also match the test groups on the test overview page

Maybe postpone this one on decisions we make in those?

So is this postponed or not?

IMO this is a bug in the UI, so it would be nice to get this fixed if it looks like those other issues are taking a while to land.

As of tests could have multiple test groups it makes sense to keep grouping with module name and just prefix (or suffix) test name with its groups

Triaging issues in simpletest.module to determine if they should be in the Simpletest Project or core.

This looks like it a simpletest module issue, changing the project.