Comments

geek-merlin created an issue. See original summary.

geek-merlin’s picture

Input from folks with more expirience in such tests appreciated!
How does core and/or other modules do this?

geek-merlin’s picture

Looked a bit into \Drupal\FunctionalJavascriptTests\JSWebAssert and the proposal to use XPath seems to make most sense to me.

spokje’s picture

Going to make a stab at it tomorrow.

joachim’s picture

XPath can be pretty hard to read.

Would using Mink's 'named' selector here work with the HTML we have? -- see https://symfonycasts.com/screencast/behat/finding-elements#find-via-the-...

spokje’s picture

Thanks @joachim, that's certainly worth a try!

I'm waiting until these test nitpicks land before I'm going in.

spokje’s picture

geek-merlin’s picture

> XPath can be pretty hard to read.

Good point. If we need them, we should at least have link name comments for maintainability.

I'd prefer the name selector (text/title/alt) too. But in our case we'll by designe have several "Title"s. Can we discriminate them? Something like $session->getXpathFragment('/path/to/second/ief')->findName('Title')?

geek-merlin’s picture

Thought and researched a bit.

Some selenium tutorials speak about "relative selectors" beginning with "//" that "start in the middle of a document". That is plain nonsense in xpath terms, as "//" is the any-descentant syntax.

geek-merlin’s picture

But it's quite simple to have something like getElementByNthContextAndLabelText($cssClass, $index, $label) that spits out xpath like //*[contains(concat(" ", normalize-space(@class), " "), " {$cssClass} ")][{$index}]//*[@id=string(//label[.='{$label}']/@for)]. (Note that most of the complexity stems from the fact that i chose CSS class for context and we must teach XPath to parse CSS classes.)(The XPath is tested on a form via the inspector's find-by-XPath feature.)

geek-merlin’s picture

@Spokje, any news on this? Need any help (or just time ;-)?

spokje’s picture

@geek-merlin
It's time (seems to be a rare resource amongst Developers somehow...).

What needs to be done is clear, how to do it also (or at least some very good suggestion were made in the above comments).

I _hope_ to tackle it during this week, but if anybody beats me to it: It would not be a problem, I haven't assigned myself to it, so the issue is open for all and everyone :)

spokje’s picture

StatusFileSize
new57.24 KB

A week, two months, semantics...
Anyway: I finally had the time to tackle this one.

In the end I went for a solution that's slightly less fancy than proposed, by just getting something like "The second input field with label 'Title'".

Attached patch passes locally, so let's see what TestBot thinks.

Note:

1) I only transformedHTML IDs for input fields, not buttons, This with issue #2653574 in mind, where we _really_ need HTML ID agnostic input fields
2) I didn't touch SimpleWidgetTest checkEditAccess, because that largely is a test to see if input fields are _not_ there. With my setup it's hard/impossible to test if the second input out of three isn't there, since the third input becomes the second. (Hopes that makes some sense to somebody...)

spokje’s picture

Status: Active » Needs review
spokje’s picture

Not sure why but /inline_entity_form/tests/src/FunctionalJavascript/ComplexWidgetRevisionsTest.php isn't triggered by TestBot, I made changes to that file as well, and they pass locally.

geek-merlin’s picture

@Spokje Impressing work!

Now i found some time to look into this. I like the approach of generating the xpath, this makes the magic less magic.
I'll play with the xpath to try make that more specific.

geek-merlin’s picture

I took the xpath-fu of #10 and adapted it to label. Now we should get exactly the right element even in edge cases. Let's see if that holds.

geek-merlin’s picture

Cool this passes too.

I went all through this patch and everything looks good to me.

More than a hundred times pain relief like this, how hot is this...🐳

+++ b/tests/src/FunctionalJavascript/ComplexWidgetTest.php
-    $page->fillField('multi[form][inline_entity_form][entities][0][form][title][0][value]', 'Duplicate!');
-    $page->fillField('multi[form][inline_entity_form][entities][0][form][first_name][0][value]', 'Bojan');
+    $title_field_xpath = $this->getXpathForNthInputByLabelText('Title', 2);
+    $first_name_field_xpath = $this->getXpathForNthInputByLabelText('First name', 1);
+    $assert_session->elementExists('xpath', $title_field_xpath)->setValue('Duplicate!');
+    $assert_session->elementExists('xpath', $first_name_field_xpath)->setValue('Bojan');
     $create_node_button->press();
spokje’s picture

@geek-merlin Nice stuff! I should have made that generic approach myself, but after spending hours of endless search-replace-retest on each individual test I somehow lost focus there.

All's green, let me have a quick review and if all's Ok (which it certainly looks like it is!), I'll make this RTBC

spokje’s picture

@geek-merlin Alas, nothing is ever easy...

Currently the test-class ComplexWidgetRevisionsTest isn't triggered by TestBot (See here).

The changes do pass locally, but I think we need to fix that first.
I've created this issue for that.

If that passes green and is committed, we can reroll your patch #17, if that passes green, I'm happy to RTBC that.
(And _then_ we can finally tackle this issue, which was what I came in for, many, many moons ago)

spokje’s picture

3118610 came back green.

spokje’s picture

geek-merlin’s picture

StatusFileSize
new56.72 KB

> And _then_ we can finally tackle this issue, which was what I came in for, many, many moons ago

Aah, i remember dimly... Hey, passing the marshmallow test is said to be a predictor for a good life. ;-)

I committeed the other issue, thanks for that! And here's a re-roll.

spokje’s picture

Status: Needs review » Reviewed & tested by the community

Hey, passing the marshmallow test is said to be a predictor for a good life. ;-)

Now you got me wondering if that is a good life _now_, or two good lives later...

Anyway: ComplexWidgetRevisionsTest is now included in the tests, all green: RTBC for me :)

  • geek-merlin committed e425dd5 on 8.x-1.x
    Issue #3103609 by geek-merlin, Spokje: Make tests (more) HTML ID...
geek-merlin’s picture

Status: Reviewed & tested by the community » Fixed

So finally that's in. Champagne! And marshmallows. 🍾🥂

Status: Fixed » Closed (fixed)

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