Problem/Motivation

Currently there are no Javascript tests that prove that contextual links work. This make it very hard for the Settings tray module and any other module that provides links via contextual links to write Javascript tests that prove there links do what they are suppose to.

Proposed resolution

Write Javascript tests for contextual links.
Perhaps create a JS Cotnextual links testing trait so that other modules can easily test their own links.

Remaining tasks

Write the tests.

User interface changes

None

API changes

None

Data model changes

None

CommentFileSizeAuthor
#15 2821724-15.patch10.19 KBtedbow
#15 interdiff-13-15.txt1021 bytestedbow
#13 2821724-13.patch10.26 KBtedbow
#7 2821724-contextual-7.patch6.59 KBtim.plunkett
#7 2821724-contextual-7-interdiff.txt4.81 KBtim.plunkett
#6 interdiff-2821724-4-6.txt586 bytestedbow
#6 2821724-6.patch6.49 KBtedbow
#4 2821724-4.patch6.5 KBtedbow
#2 2821724-2.patch6.48 KBtedbow

Comments

tedbow created an issue. See original summary.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Status: Active » Needs review
StatusFileSize
new 2821724-2.patch6.48 KB

Ok here is patch that test contextual links. It tests clicking "Edit" in the toolbar to show contextual links and without toolbar.

To test without toolbar I am forcing the button that shows contextual links to be visible. I don't see a way around this.

It also creates a trait so that other modules like Settings Tray don't have to redo the work.

Status: Needs review » Needs work

The last submitted patch, 2: 2821724-2.patch, failed testing.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Status: Needs work » Needs review
StatusFileSize
new 2821724-4.patch6.5 KB

Forgot to add @group

Status: Needs review » Needs work

The last submitted patch, 4: 2821724-4.patch, failed testing.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Status: Needs work » Needs review
StatusFileSize
new 2821724-6.patch6.49 KB
new interdiff-2821724-4-6.txt586 bytes

Copy/paste error in .info.yml file wrong package.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
StatusFileSize
new 2821724-contextual-7-interdiff.txt4.81 KB
new 2821724-contextual-7.patch6.59 KB

I spent too much time figuring out that GastonJS is busted, and mouseOver() just doesn't work as expected.
In doing that debugging, I refactored the trait a bit, I think this is more readable.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented

@tim.plunkett thanks for trying to get it work with mouseOver. Yes I had no luck either.

The refactor looks good.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented

Seems like the bug is https://github.com/jcalderonzumba/gastonjs/issues/19, #2773791: Clicking elements with children in Javascript tests throws a GastonJS exception adds a workaround for this.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented

@tim.plunkett re: #9
I just checked that work around yes it does fix this issue, though you would have to slightly alter our test to use it.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
StatusFileSize
new 2821724-13.patch10.26 KB

Rerolled and also removed the duplicate functions in the Settings Tray module tests.

Status: Needs review » Needs work

The last submitted patch, 13: 2821724-13.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-13-15.txt1021 bytes
new 2821724-15.patch10.19 KB

Ok enabling contextaul_test module broke the existing test function testContextualLinksVisibility()

So now only enabling that module in the new test function.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs review » Reviewed & tested by the community

I think this is ready to go. Thanks @tedbow for your persistence.

  • larowlan committed 5c31080 on 8.5.x 
    Issue #2821724 by tedbow, tim.plunkett: Create Javascript Tests for...

  • larowlan committed d18903c on 8.4.x 
    Issue #2821724 by tedbow, tim.plunkett: Create Javascript Tests for...
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Version: 8.5.x-dev » 8.4.x-dev
Status: Reviewed & tested by the community » Fixed

Fixed on commit

diff --git a/core/modules/contextual/tests/src/FunctionalJavascript/ContextualLinksTest.php b/core/modules/contextual/tests/src/FunctionalJavascript/ContextualLinksTest.php
index 15edb62..a24d2ba 100644
--- a/core/modules/contextual/tests/src/FunctionalJavascript/ContextualLinksTest.php
+++ b/core/modules/contextual/tests/src/FunctionalJavascript/ContextualLinksTest.php
@@ -80,4 +80,5 @@ public function testContextualLinksClick() {
     $this->clickContextualLink('#block-branding', 'Test Link', FALSE);
     $this->assertSession()->pageTextContains('Everything is contextual!');
   }
+
 }

Committed as 5c31080 and pushed to 8.5.x
Cherry-picked as d18903c and pushed to 8.4.x

Status: Fixed » Closed (fixed)

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