Convert/Modernise FlagContextualLinksTest

The bad news is I have had to completely rewrite the tests, while trying to retain all the individual checks.
The good news is that now we can click about like a user ... the intent behind testing is much easier to discern.

So in conversation language this is what we are testing

Define a flag for node content which will show up in a contextual pull out box.
Visit an article ..
Open up the contextual links
verify that the expected html elements are rendered to the screen.
toggle the flag
verify the expected contextual link as html elements changes accordingly.
login as another user.
verify the html structure found by another user is unaffected by the action of the toggling.

I have the basic structure coded .... now I must fix the bugs :)... posting early.

curiouser and curiouser said alice.....

I made the last patch using phantomjs and phpunit in the conventional manner.
This is not the first FunctionalJavascript test in this module either

I am at a loss to explain the "No tests found" error.
But for the moment I am going to get tests locally passing ... and then try and found why testbot is unhappy!

Thank you both, I appreciate the guidance.

So there are 2 pressing problems
1) local tests fail
2) The "No tests found" problem.

My changes here do nothing to sort either! ... what follows is what I found while looking over the code.

The word link is used throughout the testing .. in the cold light of day the word anchor is distracting and implies a difference - which just isn't there.

I have dropped a use of the FlagWithFormTrait ......
We have a nonstandard 2-page form and so in the broad scope of the module testing a test that that goes through the UI is essential. BUT I want to limit the scope of this contextual links test

.. get in ... test the contextual links ... and get out.

In that context it is computationally more efficient to create the flag programmatically.

Testing locally I was having a reproducibility bug... the test error were difference on successive test runs.
Creating the flag programmatically seems to make the bug reporting consistent... as usual I may be blindly stepping over the solution to my own problems...

Lastly I have fixed a few ultra trivial phpcs nits.

more from me soon.

Just making notes for myself ... so I can come back if I need to debug other functionalJavascript tests

Things I need to fix.

->click() is appropriate action on link ... ->press() is for buttons!

After reading some official mink documentation where I use assertTrue() on Element->isValid() it is an inferior code pattern

I need to start using this pattern ... I can see a massive improvement in performance when I do

    $flag_placeholder = $page->find('css', 'div[data-contextual-id="' . $flag_contextual_links_id . '"]');
    $this->assertNotNull($flag_placeholder, format_string('Contextual link placeholder with id @id exists (flag).', ['@id' => $flag_contextual_links_id]));

also after reading

https://www.phase2technology.com/blog/ajax-testing-drupal-8

./vendor/bin/phpunit -c core/phpunit.xml --printer="\Drupal\Tests\Listeners\HtmlOutputPrinter" ./modules/flag/tests/src/FunctionalJavascript/FlagContextualLinksTest.php

This is a useful test initiator for dumping step by step page screen dumps (as HTML file)

I am having to balance my time with other things ... I am still working on this ... but I have other priorities atm :(

Just for the record .... debugging has shown an issue

I can explain why the first ->press() of a button in on a NULL object!

the HTML page dumps exposes a problem with one of my assumptions.

When the page renders a unpopulated placeholder is shown on the page
without the presence of the button I am trying to ->press() ...

This is not a working solution yet...

But I am slowly working out the kinks...

1)

The first rule of contextual links club is "Don't talk about contextual links club"

The second rule is "Set the permissions correctly -- dummy" ( which I have fixed in this patch )

The third and fourth rule of contextual link club elude me as that pesky button I want to click does not appear on the screen!

2) I have streamlined the assertions into something cleaner.

assertPlaceholderMatches() becomes assertLinkMatches()

So the contextual link icon now appears on the screen.

The third rule was ... "include and plumb in the toolbar module correctly!"

Now I see this issue :-

here is the error in full

Before I can click on the flag action link ....
I must click on the contextual links icon ... wait for the panel to become visible and then click the link.
[quite understandably mink will only allow visible links to be clicked.]

1) Drupal\Tests\flag\FunctionalJavascript\FlagContextualLinksTest::testFlagLinks
Zumba\GastonJS\Exception\BrowserError: There was an error inside the PhantomJS portion of GastonJS.
This is probably a bug, so please report it:
click
html.touchevents.details.js body.toolbar-tray-open.toolbar-horizontal.toolbar-fixed.user-logged-in.path-node.page-node-type-article div.layout-container main div.layout-content div.region.region-content article.contextual-region.node.node--type-article.node--promoted.node--view-mode-full footer.node__meta article.profile div.js-form-item.form-item.js-form-type-item.form-type-item.js-form-item-.form-item-.form-no-label

/Users/martin/sites/drupal/vendor/jcalderonzumba/gastonjs/src/Browser/BrowserBase.php:122
/Users/martin/sites/drupal/vendor/jcalderonzumba/gastonjs/src/Browser/BrowserBase.php:99
/Users/martin/sites/drupal/vendor/jcalderonzumba/gastonjs/src/Browser/BrowserMouseEventTrait.php:17
/Users/martin/sites/drupal/vendor/jcalderonzumba/mink-phantomjs-driver/src/MouseTrait.php:31
/Users/martin/sites/drupal/vendor/behat/mink/src/Element/NodeElement.php:153
/Users/martin/sites/drupal/vendor/behat/mink/src/Element/NodeElement.php:161
/Users/martin/sites/drupal/modules/flag/tests/src/FunctionalJavascript/FlagContextualLinksTest.php:150

Just looking over the test report, for #16

https://dispatcher.drupalci.org/job/drupal8_contrib_patches/10427/console

1)

I think I may have found the bug behind the "No tests found issue.."

[ Me: looks up innocently ... like I could ever he held responsible for my own mistakes...]

The work of the pluralization gremlin was far greater than first thought.

Fatal error: Trait 'Drupal\flag\Tests\FlagPermissionsTrait' not found in /var/www/html/modules/contrib/flag/tests/src/Functional/ShowOnEntityFormTest.php on line 17

[Me: puts shameface back in the drawer.]

Hopefully testbot should now see the errors I see locally.

2) I have also found a minor test infrastructure issue.

20:59:06 ERROR: the "Drupal" coding standard is not installed. The installed coding standards are Zend, Squiz, PSR2, PSR1, PHPCS, PEAR and MySource

Is there a issue queue for our test infrastructure? I would like to file a bug report ...

created an issue to get #17.2 resolved.

Fixed the ultra trivial test error.

After looking at the code in PhantomJS Mink and GastonJS ... I have a suspicion as to what is blowing up with an internal error.

The click() handler converts the css selector into a xpath selector... I think it does a bad job and things go screwy

My take on this is the data-contextual-id value appears to be properly escaped for use a HTML attribute... but the converter is broken

<div data-contextual-id="node:node=1:changed=1501867577&amp;flag_keys=test_label_123-flag&amp;langcode=en" class="contextual" role="form">

Over the weekend I will try relax the css selector to still uniquely identify the contextual-links button without using the data-contextual-id value.

My suspicions in #21 are incorrect

    $button = $this->getSession()->getPage()->find('css', 'div.contextual button');
    $button->press();

I have stripped down the css selector to something simple
here is the phantomJs output the xpath is simple correct and a HTML element is being identified.
The response identifies an element with an id of 2

REQUEST: {"name":"find","args":["xpath","(\/\/html\/descendant-or-self::div[@class and contains(concat(' ', normalize-space(@class), ' '), ' contextual ')]\/descendant-or-self::*\/button)[1]"]}

RESPONSE: {
    "response": {
        "page_id": 7,
        "ids": [
            2
        ]
    }
}

Sorry for the distraction

just marshalling my thoughts while I look for answers

Went looking for similar code in core -- to see what I am doing wrong

Drupal\Tests\contextual\FunctionalJavascript\EditModeTest

is a functional javascript test that pushes the main context 'edit' button on the toolbar

  /**
   * Presses the toolbar edit mode.
   */
  protected function pressToolbarEditButton() {
    $edit_button = $this->getSession()->getPage()->find('css', '#toolbar-bar div.contextual-toolbar-tab button');
    $edit_button->press();
  }

and checks the button has functioned

  /**
   * Asserts that the correct message was announced when entering edit mode.
   */
  protected function assertAnnounceEditMode() {
    $web_assert = $this->assertSession();
    // Wait for contextual trigger button.
    $web_assert->waitForElementVisible('css', '.contextual trigger');
    $web_assert->elementContains('css', static::ANNOUNCE_SELECTOR, 'Tabbing is constrained to a set of');
    $web_assert->elementNotContains('css', static::ANNOUNCE_SELECTOR, 'Tabbing is no longer constrained by the Contextual module.');
  }

I think we need something similar.

1) composer outdated

yields this fragment

guzzlehttp/guzzle                     6.2.3              6.3.0              Guzzle is a PHP HTTP client library
jcalderonzumba/gastonjs               v1.0.2             v1.2.0             PhantomJS API based server for webpage automation
jcalderonzumba/mink-phantomjs-driver  v0.3.1             v0.3.3             PhantomJS driver for Mink framework

so things are surprisingly up to date.

2) Went looking for core issues where mink is misbehaving I am not sure if it is related.

I am going to have to curtail my attempts to modernise this test a little bit.

previously... the test

emulated the opening of contextual panels and the clicking links by visiting the relevant pages and then inspecting the Json response directly.

looking at the stack trace when you click on the links/buttons it flows to

Zumba\GastonJS\Browser\BrowserBase::command()

which is not setup to handle the json response.

[ Me: Finds reverse gear and shifts my position in the seat so I can see directly out of the rear window.... BEEP BEEP BEEP ]

I have found .. a good/acceptable way of handling a way the emulation of clicking

The tests pass ... but only because I have currently commented out calls to assertLinkMatches()
Otherwise I am happy with the code structure ... it is 95% complete.

Those asserts fail because ... well I am in the process of plumbing in the plugin system so those lines will work.

On a minor note... I deleted my attempts to spin off helper methods into

FunctionalJavascript\FlagTestBase

So this is a request for help/information

I have googled the error message

StackOverflow has a recurring thread that says for PHPUnit testing occasionally serializes the global state...
and if there are any closure there then things go wrong ....like this...

As you can see from the interdiff the stackoverflow answers suggest adding the following annotations

@backupGlobals disabled
@runInSeparateProcess

Clearly this does not work ... yet. Drupal is run on an ancient phpunit version
in the context of Drupal8 does anyone know what the answer is?

The error is a bit obscure ... butt happens on this line

    $computed_link = $flag_link_type_plugin->getAsLink($flag, $node);

I have simplified the patch

In #12 I dropped the use of FlagWithFormTrait...that makes the movement of the Trait into the proper directory structure out of scope.

In previous patch assertLinkMatches() used a clunky code pattern to inspect the URL
this is a more standard way to doing the same thing...

    $flag_url = Url::fromRoute("flag.action_link_{$action}", [
      'flag' => $flag->id(),
      'entity_id' => $node->id(),
    ]);

This new code pattern still has the same error ....

PHPUnit_Framework_Exception: PHP Fatal error: Uncaught Exception: Serialization of 'Closure' is not allowed in

Just posting a update ....

Normally I would be filing a branch blocker ....

locally FlagContextualLinkTest is failing on the the current unpatched Drupal 8.5.x and Flag 8.x-4.x combo

Specifically this commit for Flag

commit e208139849768f821e196dd5f154fee2019b7080
Author: beram
Date: Sat Jul 29 19:33:25 2017 -0500

Issue #2732065 by beram, Blanca.Esqueda, TheDucksLover, matthew.perry: Fixed flag links redirect to the system path rather than the actual path the user was on.

and this commit for Drupal8

commit 48a8e5307f6236622f46cb54bef6c24e17757d92
Author: Lee Rowlands
Date: Fri Sep 1 19:02:44 2017 +1000

Issue #2278383 by dawehner, Xano, kim.pepper, twistor, jibran, Jo Fitzgerald, joelpittet, pritish.kumar, Fabianx, Crell, larowlan, alexpott, catch, Wim Leers: Create an injectible service for drupal_set_message()

But I am in the middle of converting the failing function... I am time limited at the moment.

In recent weeks there have been lots of changes to WebTest and associated traits - which this test depends so that is the likely source of gremlins.

Anyways if you want me to post a branch blocker to separate out the issues .. just let me know.

Anyways if you want me to post a branch blocker to separate out the issues .. just let me know.

In the end I split out the branch blocker. .. it is a one line fix ...I am attaching a link.

That makes this issue pass tests.. and I think the conversion is complete.... unassinging

Drupal8 core has been tightening up its coding standard check ... which is dumping lots of nits into the test report.

this patch has less nits

I will try to look at why I have introduce a serialization issue... soon

Yay I found the cause of the serialization thing... it is not worth describing here ... given the level of the changes that immediately followed... sorry for the delay.

regarding this comment

    // Simulate clicking on the flag contextual link.
-    //
-    // Zumba\GastonJS\Browser does not support the handling of JSON response
-    // objects so instead of clicking we programatically change the state of the
-    // flag and reload the page.

In the refactoring this is not longer relevant .. nothing needs to be so directly manipulated/faked ... everything can happen through the simulated browser.

The solution is :-
Click on the main contextual "Edit" button
Bring up the contextual icons.
Click through until the relevant contextual link panel appears.
Click the embedded contextual flag link..

I have also made a design decision which may be controversial so I need to justify it

previously we were making assertions about the value of the href attribute... I stopped that

It was a sign that the test was too tightly coupled to the implementation...

What the test cares about is that the flag action link can be used as part of core contextual link system.
The fact that we can click on the flag action link and then the next time we are presented with the unflag action link is enough to test flag action link irrespective of how the design chosed to do it.

Nothing but a coding standard tidy on FlagContextualLinksTest

Thanks for the prompt review and action... it is much appreciated.

socketwench++