Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-42284

Automate MDLQA-135 - Message history displays correctly

    Details

    • Testing Instructions:
      Hide

      Run the following tests (change the path in the commands)

      vendor/bin/behat --config=/DIR/behat/behat/behat.yml --name "View received messages"

      vendor/bin/behat --config=/DIR/behat/behat/behat.yml --name "View sent messages"

      Show
      Run the following tests (change the path in the commands) vendor/bin/behat --config=/DIR/behat/behat/behat.yml --name "View received messages" vendor/bin/behat --config=/DIR/behat/behat/behat.yml --name "View sent messages"
    • Affected Branches:
      MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE
    • Pull Master Branch:
      MDL-42284_behat
    • Sprint:
      FRONTEND Sprint 6
    • Sprint:
      FRONTEND Sprint 6

      Description

      As described in MDLQA-135, message history displays correctly

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            dmonllao David Monllaó added a comment -

            Hi Andrew,

            I was thinking that we could merge this new display_history.feature and the existing message/tests/behat/search_history.feature, but it is probably better to keep it like you have done it as it works really well to test both JS and non-JS.

            About the patch it looks really good, I only can say that the usage of the prefixes Given, When and Then can be improved, we want to read the scenario and know what is testing and is good to have the test split in 3 parts, using those prefixes for it, so a scenario should look like

            Given
            And
            And
            And
            ...
            When
            And
            And
            ...
            Then
            And
            And
            ...

            Show
            dmonllao David Monllaó added a comment - Hi Andrew, I was thinking that we could merge this new display_history.feature and the existing message/tests/behat/search_history.feature, but it is probably better to keep it like you have done it as it works really well to test both JS and non-JS. About the patch it looks really good, I only can say that the usage of the prefixes Given, When and Then can be improved, we want to read the scenario and know what is testing and is good to have the test split in 3 parts, using those prefixes for it, so a scenario should look like Given And And And ... When And And ... Then And And ...
            Hide
            dmonllao David Monllaó added a comment -

            I forgot to mention:

            • The commit msg should include the MDLQA issue number
            • It should be backported as long as it is possible (I guess it could simply be cherry-picked, not 100% sure tough)
            Show
            dmonllao David Monllaó added a comment - I forgot to mention: The commit msg should include the MDLQA issue number It should be backported as long as it is possible (I guess it could simply be cherry-picked, not 100% sure tough)
            Hide
            andyjdavis Andrew Davis added a comment -

            Ok, I think I have it. I have removed the JS version as there is no JS used in the messaging UI. I will backport this once I have it right. Putting this back up for peer review.

            Show
            andyjdavis Andrew Davis added a comment - Ok, I think I have it. I have removed the JS version as there is no JS used in the messaging UI. I will backport this once I have it right. Putting this back up for peer review.
            Hide
            dmonllao David Monllaó added a comment -

            Hi Andrew, the use of Given/When/Then is not correct, please, follow the provided example or any other .feature file in the codebase

            Show
            dmonllao David Monllaó added a comment - Hi Andrew, the use of Given/When/Then is not correct, please, follow the provided example or any other .feature file in the codebase
            Hide
            andyjdavis Andrew Davis added a comment -

            Putting this back up for peer review. I could split the "View sent messages" scenario into two scenarios but that seems unnecessary. Let me know if I should do that.

            Show
            andyjdavis Andrew Davis added a comment - Putting this back up for peer review. I could split the "View sent messages" scenario into two scenarios but that seems unnecessary. Let me know if I should do that.
            Hide
            dmonllao David Monllaó added a comment -

            Thanks Andrew, it looks good.

            Note that https://github.com/andyjdavis/moodle/compare/master...MDL-42284_behat#diff-3cdc0560fe0be956fa6b9eb716b1a868R34 is a When and is part of the Then section so it should be an And (And extends the last non-And-prefixed step, so it would extend Then in this case) Is important to have the tests split in 3 parts, but I don't want to be too picky about it as everybody is busy with other issues, so feel free to send to integration.

            Show
            dmonllao David Monllaó added a comment - Thanks Andrew, it looks good. Note that https://github.com/andyjdavis/moodle/compare/master...MDL-42284_behat#diff-3cdc0560fe0be956fa6b9eb716b1a868R34 is a When and is part of the Then section so it should be an And ( And extends the last non-And-prefixed step, so it would extend Then in this case) Is important to have the tests split in 3 parts, but I don't want to be too picky about it as everybody is busy with other issues, so feel free to send to integration.
            Hide
            andyjdavis Andrew Davis added a comment -

            Submitting for integration.

            Show
            andyjdavis Andrew Davis added a comment - Submitting for integration.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Thanks Andrew - this has been integrated now. 25 + master only as 24 doesn't have behat of course.

            Show
            samhemelryk Sam Hemelryk added a comment - Thanks Andrew - this has been integrated now. 25 + master only as 24 doesn't have behat of course.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Tested during integration review.

            Show
            samhemelryk Sam Hemelryk added a comment - Tested during integration review.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            FYI: related MDLQA-135 has been moved from MDLQA-1 to MDLQA-5249 (bag of behat-converted tests). Thanks!

            Show
            samhemelryk Sam Hemelryk added a comment - FYI: related MDLQA-135 has been moved from MDLQA-1 to MDLQA-5249 (bag of behat-converted tests). Thanks!
            Hide
            poltawski Dan Poltawski added a comment -

            Congratulations - this issue has been included in Moodle and is now available on our git mirrors and shortly will become available on the download servers shortly.

            Show
            poltawski Dan Poltawski added a comment - Congratulations - this issue has been included in Moodle and is now available on our git mirrors and shortly will become available on the download servers shortly.

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  11/Nov/13

                  Agile