CommentFileSizeAuthor
#41 menu-multilingual-context-6.patch3.9 KBmatsbla
#40 menu-multilingual-context-5.patch4.1 KBmatsbla
#39 menu-multilingual-context-4.patch3.43 KBmatsbla
#38 menu-multilingual-context-3.patch3.38 KBmatsbla
#37 menu-multilingual-context-2.patch3.23 KBmatsbla
#36 menu-multilingual-context.patch3.23 KBmatsbla
#32 3037173-32.patch3.23 KBjeroent
#26 menu-multilingual-views-menu-links.patch6.65 KBmatsbla
#23 menu-multilingual-menu-block-test-14.patch14.32 KBmatsbla
#21 menu-multilingual-menu-block-test-13.patch11.22 KBmatsbla
#20 menu-multilingual-menu-block-test-13.patch0 bytesmatsbla
#19 menu-multilingual-menu-block-test-12.patch11.22 KBmatsbla
#18 menu-multilingual-menu-block-test-11.patch10.43 KBmatsbla
#16 menu-multilingual-menu-block-test-10.patch32.06 KBjeroent
#16 interdiff-3037173-9-10.txt643 bytesjeroent
#15 interdiff-3037173-8-9.txt2.17 KBjeroent
#15 menu-multilingual-menu-block-test-9.patch31.37 KBjeroent
#14 menu-multilingual-menu-block-test-8.patch32.14 KBmatsbla
#13 menu-multilingual-menu-block-test-7.patch6.31 KBmatsbla
#11 menu-multilingual-menu-block-test-6.patch720 bytesmatsbla
#10 menu-multilingual-menu-block-test-5.patch789 bytesmatsbla
#7 menu-multilingual-menu-block-test-4.patch31.82 KBjeroent
#7 interdiff.txt712 bytesjeroent
#6 menu-multilingual-menu-block-test-3.patch31.82 KBjeroent
#5 menu-multilingual-menu-block-test-2.patch31.13 KBjeroent
#5 interdiff.txt681 bytesjeroent
#4 menu-multilingual-menu-block-test.patch31.16 KBjeroent
create-tests-custom-menu-links.patch22.88 KBmatsbla

Comments

matsbla created an issue. See original summary.

  • matsbla committed cbf574a on 8.x-1.x 
    Issue #3037173 by matsbla: Create tests: Custom menu links
matsbla’s picture

matsbla commented
Status: Active » Needs work

Committed first tests for custom menu items.

Still need tests for view links, and integration with Menu block and Context.

jeroent’s picture

jeroent
Primary language Dutch
Location 🇧🇪
commented
Status: Needs work » Needs review
StatusFileSize
new menu-multilingual-menu-block-test.patch31.16 KB

Started working on a menu_block test.

jeroent’s picture

jeroent
Primary language Dutch
Location 🇧🇪
commented
StatusFileSize
new interdiff.txt681 bytes
new menu-multilingual-menu-block-test-2.patch31.13 KB
jeroent’s picture

jeroent
Primary language Dutch
Location 🇧🇪
commented
StatusFileSize
new menu-multilingual-menu-block-test-3.patch31.82 KB

Added composer.json file to add menu_block module.

jeroent’s picture

jeroent
Primary language Dutch
Location 🇧🇪
commented
StatusFileSize
new interdiff.txt712 bytes
new menu-multilingual-menu-block-test-4.patch31.82 KB
jeroent’s picture

jeroent
Primary language Dutch
Location 🇧🇪
commented

The tests are running now, but I still get the following error when running the tests:

The website encountered an unexpected error. Please try again later.

Drupal\Core\Config\Schema\SchemaIncompleteException: Schema errors for block.block.mainnavigation with the following errors: block.block.mainnavigation:settings.expand missing schema, block.block.mainnavigation:settings.only_translated_labels missing schema, block.block.mainnavigation:settings.only_translated_content missing schema in Drupal\Core\Config\Development\ConfigSchemaChecker->onConfigSave() (line 95 of core/lib/Drupal/Core/Config/Development/ConfigSchemaChecker.php).
jeroent’s picture

jeroent
Primary language Dutch
Location 🇧🇪
commented

Error "block.block.mainnavigation:settings.expand missing schema" is fixed in 8.x-1.x branch of menu_block module. But when using this branch, there are a couple of other issues which are solved here: https://www.drupal.org/project/menu_block/issues/3022011.

There is still a missing schema for block.block.mainnavigation:settings.only_translated_labels and block.block.mainnavigation:settings.only_translated_content.

matsbla’s picture

matsbla commented
StatusFileSize
new menu-multilingual-menu-block-test-5.patch789 bytes

I think the Menu Block tests should be the same as the base tests, as it should pass all the same tests, so we can simply extend the base test and add Menu Block to the module dependencies.

matsbla’s picture

matsbla commented
StatusFileSize
new menu-multilingual-menu-block-test-6.patch720 bytes
jeroent’s picture

jeroent
Primary language Dutch
Location 🇧🇪
commented

@matsbla,

In your patch, the menu_block module is enabled, but the main navigation is still a system_menu_block. Tests will probably pass because the menu_block module is not used.

In my patch, I moved all the helper functions to a base test class. I also added a composer.json file because the testbot has no idea where he has to download the menu_block module without it.

matsbla’s picture

matsbla commented
StatusFileSize
new menu-multilingual-menu-block-test-7.patch6.31 KB

Okay, you are right. I tested more. In the setup I tried to assert that we are using menu_block when adding new manu blocks. It seems like the form is correct. However when saving the block it is returned a status code 500, I tested more, but I'm not sure why this happens.

matsbla’s picture

matsbla commented
StatusFileSize
new menu-multilingual-menu-block-test-8.patch32.14 KB
jeroent’s picture

jeroent
Primary language Dutch
Location 🇧🇪
commented
StatusFileSize
new menu-multilingual-menu-block-test-9.patch31.37 KB
new interdiff-3037173-8-9.txt2.17 KB

Lets check with strict config schema disabled.

jeroent’s picture

jeroent
Primary language Dutch
Location 🇧🇪
commented
StatusFileSize
new interdiff-3037173-9-10.txt643 bytes
new menu-multilingual-menu-block-test-10.patch32.06 KB
jeroent’s picture

jeroent
Primary language Dutch
Location 🇧🇪
commented

So the problem is the schema. Let's see if I can find something.

matsbla’s picture

matsbla commented
StatusFileSize
new menu-multilingual-menu-block-test-11.patch10.43 KB

Okay! Maybe we could commit it with strictConfigSchema set to false for now, to get a test, and remove it in a follow-up once the schemas in menu_block is fixed.

I reworked the patch a little, I think it will be easier to maintain the tests if we keep them in 1 place, let me know what you think.

Thanks for helping with the tests!

matsbla’s picture

matsbla commented
StatusFileSize
new menu-multilingual-menu-block-test-12.patch11.22 KB

For to add the composer.json file

matsbla’s picture

matsbla commented
StatusFileSize
new menu-multilingual-menu-block-test-13.patch0 bytes
matsbla’s picture

matsbla commented
StatusFileSize
new menu-multilingual-menu-block-test-13.patch11.22 KB
jeroent’s picture

jeroent
Primary language Dutch
Location 🇧🇪
commented

@matsbla,

Code looks good!

Only thing I would change is in every drupalPostForm the label should not be translated. Klausi mentioned this once to me in #3024558-41: Username regenerated improperly after rc6 release.5

matsbla’s picture

matsbla commented
StatusFileSize
new menu-multilingual-menu-block-test-14.patch14.32 KB

Okay so removing string translations from tests

  • matsbla committed 42a7685 on 8.x-1.x 
    Issue #3037173 by matsbla, JeroenT: Create tests
matsbla’s picture

matsbla commented

Created a follow up #3039094: Make Menu Block tests use strict config schema
We are still missing tests for Context integration and views links

matsbla’s picture

matsbla commented
StatusFileSize
new menu-multilingual-views-menu-links.patch6.65 KB

Tests for views menu links

  • matsbla committed e2fb356 on 8.x-1.x 
    Issue #3037173 by matsbla, JeroenT: Create tests: Views Menu links
jeroent’s picture

jeroent
Primary language Dutch
Location 🇧🇪
commented
Issue summary: View changes
jeroent’s picture

jeroent
Primary language Dutch
Location 🇧🇪
commented
Issue summary: View changes
matsbla’s picture

matsbla commented
Issue summary: View changes
matsbla’s picture

matsbla commented
Status: Needs review » Needs work
jeroent’s picture

jeroent
Primary language Dutch
Location 🇧🇪
commented
Status: Needs work » Needs review
StatusFileSize
new 3037173-32.patch3.23 KB
jeroent’s picture

jeroent
Primary language Dutch
Location 🇧🇪
commented
Status: Needs review » Needs work

The patch I wrote depends on #3228412: Replace deprecated code in tests so we should probably wait until that gets committed.

jeroent’s picture

jeroent
Primary language Dutch
Location 🇧🇪
commented

Also, it would be great to have tests are scheduled for "Run on commit and for issues" so tests also run for merge requests.

matsbla’s picture

matsbla commented

Thanks for the effort!
Have scheduled tests for "Run on commit and for issues" as you suggested, thank you.

matsbla’s picture

matsbla commented
StatusFileSize
new menu-multilingual-context.patch3.23 KB

Reroll patch for context integration

matsbla’s picture

matsbla commented
StatusFileSize
new menu-multilingual-context-2.patch3.23 KB
matsbla’s picture

matsbla commented
StatusFileSize
new menu-multilingual-context-3.patch3.38 KB
matsbla’s picture

matsbla commented
StatusFileSize
new menu-multilingual-context-4.patch3.43 KB
matsbla’s picture

matsbla commented
StatusFileSize
new menu-multilingual-context-5.patch4.1 KB
matsbla’s picture

matsbla commented
StatusFileSize
new menu-multilingual-context-6.patch3.9 KB
matsbla’s picture

matsbla commented
Issue summary: View changes
Status: Needs work » Fixed

Lasts tests for context module added in #2956990: Settings no longer stored as third party settings

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.