Uploaded image for project: '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 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
          quen Sam Marshall added a comment -

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

          Show
          quen Sam Marshall added a comment - Tim has reviewed this minor fix, we think it's okay.
          Hide
          quen 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
          quen 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
          rajeshtaneja Rajesh Taneja added a comment -

          Thanks for fixing this Sam,

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

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

          Thank you Rajesh!

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

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

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

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

          Show
          marina Marina Glancy added a comment - Should it be backported? We usually backport behat-related changes
          Hide
          quen 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
          quen 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
          quen 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
          quen 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
          rajeshtaneja 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
          rajeshtaneja 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 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 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 Marina Glancy added a comment -

          Thanks Sam, integrated in 2.5, 2.6 and master

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

          thanks Marina! Also thanks for info about new tests.

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

          Thanks Sam,

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

          Andrew

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

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

          Show
          marina 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:
                Fix Release Date:
                12/May/14