Core uses hyphen for paths, not underscores

Thanks for the suggestion!

I completely agree, consistency is really important in a project like this.

Patches are welcome.

Good idea.

Let's do this for 8.x-1.x first, and then backport.

Updated the issue summary with instructions. Suitable for Novice so I'm leaving the tag.

I have updated the patch

fixed the issues in the test files

Hi Mile23,

I am somehow unable to add test results for the patch I have submitted. Testing results on my local are OK.

Set the issue to 'Needs review' and the patch will be sent to the testbot.

And then of course we have to 'add test' and switch the testbot to use the proper version of core.

Link to Dispatcher : https://dispatcher.drupalci.org/job/default/120636/console

Drupal CI is throwing an error ;

PHP Fatal error: Class 'Drupal\field\Tests\FieldUnitTestBase' not found in /var/www/html/modules/examples/field_permission_example/src/Tests/FieldNoteItemTest.php on line 22

Testing is dependent on : Issue #2690403 https://www.drupal.org/node/2690403

The problem here is that the test classes provided by core changed around. Drupal 8.2.x has different test classes than 8.1.x (more or less).

Also, when you switch a patch to 'needs review,' the testbot automatically tests against the latest *dev* version of Drupal core. In this case, 8.2.x. You can see it in the test result in #24.

The target version of core for the Examples project is the latest stable release, which is 8.1.0. Since we can't specify that, 8.1.x will have to do.

So if we re-start the test, and choose 8.1.x as the version of core we want to test against, we'll (probably) get a test run that at least runs, rather than erroring out.

We click on 'add test' and add it. I'll do it now. Let's see what happens.

Awesome ! I will be careful while choosing the test against version of drupal. The test has been passed, let us wait for someone to review it.

Reviewed patch core_uses_hyphen_for-2661834-24.patch. It's working fine.

#2690403: Fatal error: Class 'Drupal\field\Tests\FieldUnitTestBase' not found is committed. Yay.

Let's postpone this issue until #2686591: Consolidate fapi_example tests into one test class is committed, since we need the tests to work for the reroll.

#2686591: Consolidate fapi_example tests into one test class is in, so:

Setting to needs review, restarting the test for #24.

Merging of #2686591: Consolidate fapi_example tests into one test class has lead to removal of test files, that's why the drupalci results so (patch faild to apply), I am working to recreate this accordingly.

This patch also includes changes in FapiExampleWebTest.php

As an aside, is there some Drupal standard about the subject? I could not find any. Drupal 8 does not convert underscores to hyphens in machine names when they are part of URL. Here is an issue for content types: https://www.drupal.org/node/1574668.

I think 'be like core' is a worthy idea.

I also think it would be super awesome if we could refer to an issue or change notice explaining why core does it this way.

That said, here's a review:

+++ b/content_entity_example/src/Tests/ContentEntityExampleTest.php
@@ -227,18 +227,16 @@ class ContentEntityExampleTest extends ExamplesTestBase {
     $this->drupalLogin($web_user);
-    $entity_name = 'content_entity_example_contact';
-    $add_field_url = 'admin/structure/' . $entity_name . '_settings/fields/add-field';
+    $add_field_url = 'admin/structure/content-entity-example-contact-settings/fields/add-field';
     $this->drupalGet($add_field_url);
-    $field_name = 'test_name';
     $edit = array(
       'new_storage_type' => 'list_string',
       'label' => 'test name',
-      'field_name' => $field_name,
+      'field_name' => 'test_names',
     );

Unneeded changes. Let's just work on the hyphens and not the variables.

Make a follow-up issue if other changes are needed.

Referring to #41 and to https://www.drupal.org/node/1574668, I have done the necessary changes.

This patch works fine .. Good to go :)

This is a lot of work, thanks to all the contributors of this issue!
I have just added it to 8.x-1.x.