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

Automate MDLQA-1546 and MDLQA-1545 - blocks management

    Details

    • Story Points (Obsolete):
      8
    • Sprint:
      FRONTEND Sprint 6

      Description

      Both features can be specified in a single blocks management feature file as they have parts in common and probably the initial context set up can be shared in a background section

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            dmonllao David Monllaó added a comment -

            Hi Rossie,

            It looks good, just a few comments:

            Show
            dmonllao David Monllaó added a comment - Hi Rossie, It looks good, just a few comments: You can switch steps like https://github.com/rwijaya/moodle/compare/moodle:master...MDL-42280#diff-f62de765afae2e3720abddd272621f33R64 to Then I should see "Comments" in the "Comments" "block" Would be good to restrict some checkings to a smaller scope when using strings as common as comments https://github.com/rwijaya/moodle/compare/moodle:master...MDL-42280#diff-f62de765afae2e3720abddd272621f33R55 you can use I should see "Comments" in the "Comments" "block" for example if the comments block should appear or you can use ... in the "region-pre" "region" if not Please follow the correct indentation, see other .feature files examples The usage of Given/When/Then is not correct, you can only pass through them in this direction Given -> When -> Then Any particular reason to leave that much space between steps Seeing how long the test is and that you are merging two MDLQAs probably would make sense to split the test in 2 different scenarios according to the MDLQAs descriptions, make sure that the scenario descriptions are explaining what is the scenario testing
            Hide
            dmonllao David Monllaó added a comment -

            To split the scenario in two you might be interested in using the background section for the shared parts http://docs.behat.org/guides/1.gherkin.html#backgrounds

            Show
            dmonllao David Monllaó added a comment - To split the scenario in two you might be interested in using the background section for the shared parts http://docs.behat.org/guides/1.gherkin.html#backgrounds
            Hide
            rwijaya Rossiani Wijaya added a comment -

            Hi David,

            I updated the patch according to your suggestion. However, I'm not sure if I used "when > then" correctly.

            When you have a chance, could you please review this again?

            Thanks

            Show
            rwijaya Rossiani Wijaya added a comment - Hi David, I updated the patch according to your suggestion. However, I'm not sure if I used "when > then" correctly. When you have a chance, could you please review this again? Thanks
            Hide
            dmonllao David Monllaó added a comment -

            Hi Rossie, have you pushed your changes? I still see most (if not all) the same issues I commented about

            Show
            dmonllao David Monllaó added a comment - Hi Rossie, have you pushed your changes? I still see most (if not all) the same issues I commented about
            Hide
            rwijaya Rossiani Wijaya added a comment -

            Sorry, I just pushed the changes.

            Show
            rwijaya Rossiani Wijaya added a comment - Sorry, I just pushed the changes.
            Hide
            dmonllao David Monllaó added a comment -

            Looks very good Rossie, thanks, only 2 minor issues:

            Show
            dmonllao David Monllaó added a comment - Looks very good Rossie, thanks, only 2 minor issues: For what I see you can expand the Background section until https://github.com/rwijaya/moodle/compare/moodle:master...MDL-42280#diff-f62de765afae2e3720abddd272621f33R50 as those steps are repeated in both scenarios The Given/When/Then prefixes separates the scenarios in 3 parts we should not return to When after a Then, the prefixes only goes in one direction (I've just updated the MDocs page http://docs.moodle.org/dev/Acceptance_testing#Writing_features point #5.4 to give more info about it) From https://github.com/rwijaya/moodle/compare/moodle:master...MDL-42280#diff-f62de765afae2e3720abddd272621f33R53 for example, all should be And, same in the other scenario https://github.com/rwijaya/moodle/compare/moodle:master...MDL-42280#diff-f62de765afae2e3720abddd272621f33R76
            Hide
            rwijaya Rossiani Wijaya added a comment -

            Thanks David.

            Updated the patch and sending for integration review.

            Show
            rwijaya Rossiani Wijaya added a comment - Thanks David. Updated the patch and sending for integration review.
            Hide
            poltawski Dan Poltawski added a comment -

            Hi Rosie, I think this should be back ported to 2.5?

            Show
            poltawski Dan Poltawski added a comment - Hi Rosie, I think this should be back ported to 2.5?
            Hide
            rwijaya Rossiani Wijaya added a comment -

            Yes, it should.

            I backported the patch to 2.5 and tested. Behat test passed for 2.5.

            Show
            rwijaya Rossiani Wijaya added a comment - Yes, it should. I backported the patch to 2.5 and tested. Behat test passed for 2.5.
            Hide
            poltawski Dan Poltawski added a comment -

            Hi Rosie,

            Looks good on 25, but running on master at the moment i'm getting fails:

            .................................F.................................... 70
            .......................................F.............................. 140
            ............................
             
            (::) failed steps (::)
             
            01. Xpath matching locator "//*[@id='region-pre']/descendant::div[contains(concat(' ', normalize-space(@class), ' '), ' block ')]" not found.
                In step `And I should see "Comments" in the "//*[@id='region-pre']/descendant::div[contains(concat(' ', normalize-space(@class), ' '), ' block ')]" "xpath_element"'. # behat_general::assert_element_contains_text()
                From scenario `Add and configure a block throughtout the site'.                                                                                                       # /Users/danp/moodles/im/moodle/blocks/tests/behat/configure_block_throughout_site.feature:8
                Of feature `Add and configure blocks throughout the site'.                                                                                                            # /Users/danp/moodles/im/moodle/blocks/tests/behat/configure_block_throughout_site.feature
             
            02. Css matching locator "#region-post" not found.
                In step `And I should see "Comments" in the "#region-post" "css_element"'.                                                                                            # behat_general::assert_element_contains_text()
                From scenario `Block settings can be modified so that a block can be hidden or moved'.                                                                                # /Users/danp/moodles/im/moodle/blocks/tests/behat/manage_blocks.feature:64
                Of feature `Block appearances'.                                                                                                                                       # /Users/danp/moodles/im/moodle/blocks/tests/behat/manage_blocks.feature
             
            8 scenarios (6 passed, 2 failed)
            168 steps (166 passed, 2 failed)
            5m16.962s

            Show
            poltawski Dan Poltawski added a comment - Hi Rosie, Looks good on 25, but running on master at the moment i'm getting fails: .................................F.................................... 70 .......................................F.............................. 140 ............................   (::) failed steps (::)   01. Xpath matching locator "//*[@id='region-pre']/descendant::div[contains(concat(' ', normalize-space(@class), ' '), ' block ')]" not found. In step `And I should see "Comments" in the "//*[@id='region-pre']/descendant::div[contains(concat(' ', normalize-space(@class), ' '), ' block ')]" "xpath_element"'. # behat_general::assert_element_contains_text() From scenario `Add and configure a block throughtout the site'. # /Users/danp/moodles/im/moodle/blocks/tests/behat/configure_block_throughout_site.feature:8 Of feature `Add and configure blocks throughout the site'. # /Users/danp/moodles/im/moodle/blocks/tests/behat/configure_block_throughout_site.feature   02. Css matching locator "#region-post" not found. In step `And I should see "Comments" in the "#region-post" "css_element"'. # behat_general::assert_element_contains_text() From scenario `Block settings can be modified so that a block can be hidden or moved'. # /Users/danp/moodles/im/moodle/blocks/tests/behat/manage_blocks.feature:64 Of feature `Block appearances'. # /Users/danp/moodles/im/moodle/blocks/tests/behat/manage_blocks.feature   8 scenarios (6 passed, 2 failed) 168 steps (166 passed, 2 failed) 5m16.962s
            Hide
            poltawski Dan Poltawski added a comment -

            My bad, this was caused by me running in clean.

            Show
            poltawski Dan Poltawski added a comment - My bad, this was caused by me running in clean.
            Hide
            poltawski Dan Poltawski added a comment -

            Integrated to msater and 25 - thanks Rosie

            Show
            poltawski Dan Poltawski added a comment - Integrated to msater and 25 - thanks Rosie
            Hide
            ankit_frenz Ankit Agarwal added a comment -

            works as described.

            thanks

            Show
            ankit_frenz Ankit Agarwal added a comment - works as described. thanks
            Hide
            damyon Damyon Wiese added a comment -

            Here lies 52 bugs.
            All fixed or swept under a rug.
            If they come back one day,
            To our dismay,
            We all will feel quite un-smug.

            Thanks for the reporting/fixing/testing on this issue. It has been sent upstream.

            Show
            damyon Damyon Wiese added a comment - Here lies 52 bugs. All fixed or swept under a rug. If they come back one day, To our dismay, We all will feel quite un-smug. Thanks for the reporting/fixing/testing on this issue. It has been sent upstream.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            FYI: related MDLQA-1545 and MDLQA-1546 have been moved from MDLQA-1 to MDLQA-5249 (bag of behat-converted tests). Thanks!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - FYI: related MDLQA-1545 and MDLQA-1546 have been moved from MDLQA-1 to MDLQA-5249 (bag of behat-converted tests). Thanks!

              People

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

                Dates

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

                  Agile