Moodle
  1. Moodle
  2. MDL-44837

Behat: 'dialogue' selector only works for confirm dialogues

    Details

      Description

      The "dialogue" selector in Behat is currently only used in one place (the quiz test). Perhaps unsurprisingly, it doesn't work properly if anyone tries to use it anywhere else - the xpath statement only matches confirm dialogues, because it relies on an <h1> element which is not included in default Moodle notification dialogues.

      Also the way it's written is kind of horrible even by xpath standards (basically, previously it was like "element a/required child b/parent a" so it has element a twice; it should be "element a[required child b]"). And it's written all on one line so as to make it hard to read.

      I'll submit a fix for this.

        Gliffy Diagrams

          Activity

          Show
          CiBoT added a comment - Results for MDL-44837 Remote repository: https://github.com/sammarshallou/moodle.git Remote branch MDL-44837 -master to be integrated into upstream master Executed job http://social.srv.in.moodle.com/job/Precheck%20remote%20branch/2493 Details: http://social.srv.in.moodle.com/job/Precheck%20remote%20branch/2493/artifact/work/smurf.html
          Hide
          Sam Marshall added a comment -

          Tim has reviewed this minor fix, we think it's okay.

          Show
          Sam Marshall added a comment - Tim has reviewed this minor fix, we think it's okay.
          Hide
          Sam Marshall added a comment -

          note: use of this (in the case where it failed previously) will also be included in Behat tests for MDL-44070 when I finish those.

          Show
          Sam Marshall added a comment - note: use of this (in the case where it failed previously) will also be included in Behat tests for MDL-44070 when I finish those.
          Hide
          Rajesh Taneja added a comment -

          Thanks for fixing this Sam,

          I took liberty to look at your patch and it looks good.

          Show
          Rajesh Taneja added a comment - Thanks for fixing this Sam, I took liberty to look at your patch and it looks good.
          Hide
          Sam Marshall added a comment -

          Thank you Rajesh!

          Show
          Sam Marshall added a comment - Thank you Rajesh!
          Hide
          CiBoT added a comment -

          Moving this issue to current integration cycle, will be reviewed soon. Thanks for the hard work!

          Show
          CiBoT added a comment - Moving this issue to current integration cycle, will be reviewed soon. Thanks for the hard work!
          Hide
          Marina Glancy added a comment -

          Should it be backported? We usually backport behat-related changes

          Show
          Marina Glancy added a comment - Should it be backported? We usually backport behat-related changes
          Hide
          Sam Marshall added a comment -

          Not sure - this bug doesn't break current behat tests, because none use dialogs (except the quiz one that happens to be a confirm dialog).

          Is there any prospect of adding more behat tests to the stable branches? If so then yes I think it should be backported. Otherwise probably not needed, but can if you want!

          Show
          Sam Marshall added a comment - Not sure - this bug doesn't break current behat tests, because none use dialogs (except the quiz one that happens to be a confirm dialog). Is there any prospect of adding more behat tests to the stable branches? If so then yes I think it should be backported. Otherwise probably not needed, but can if you want!
          Hide
          Sam Marshall added a comment -

          I have created the stable version branches (the cherry-pick worked cleanly in both cases), so please do backport if appropriate.

          Show
          Sam Marshall added a comment - I have created the stable version branches (the cherry-pick worked cleanly in both cases), so please do backport if appropriate.
          Hide
          Rajesh Taneja added a comment -

          IMHO, it's a nice change and would be good to have in 25 and 26 as well.
          I will run behat locally with this patch to make sure it doesn't break anything.

          Show
          Rajesh Taneja added a comment - IMHO, it's a nice change and would be good to have in 25 and 26 as well. I will run behat locally with this patch to make sure it doesn't break anything.
          Hide
          Marina Glancy added a comment -

          Raj, I have run already the @mod_quiz test on all branches with cherry-picked commit, so I'll just integrate it.
          Sam, yes we backport all new tests to current branches (unless they test new functionality)

          Show
          Marina Glancy added a comment - Raj, I have run already the @mod_quiz test on all branches with cherry-picked commit, so I'll just integrate it. Sam, yes we backport all new tests to current branches (unless they test new functionality)
          Hide
          Marina Glancy added a comment -

          Thanks Sam, integrated in 2.5, 2.6 and master

          Show
          Marina Glancy added a comment - Thanks Sam, integrated in 2.5, 2.6 and master
          Hide
          Sam Marshall added a comment -

          thanks Marina! Also thanks for info about new tests.

          Show
          Sam Marshall added a comment - thanks Marina! Also thanks for info about new tests.
          Hide
          Andrew Nicols added a comment -

          Thanks Sam,

          This highlighted some issues with MDL-32729, which are now addressed.

          Andrew

          Show
          Andrew Nicols added a comment - Thanks Sam, This highlighted some issues with MDL-32729 , which are now addressed. Andrew
          Hide
          Marina Glancy added a comment -

          Thanks for your hard work. Your code is now part of Moodle.

          Show
          Marina Glancy added a comment - Thanks for your hard work. Your code is now part of Moodle.

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: