Moodle
  1. Moodle
  2. MDL-43332

Automate MDLQA-317 Grades Automated functional tests

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.5.4, 2.6.1, 2.7
    • Fix Version/s: 2.5.5, 2.6.2
    • Component/s: Gradebook
    • Labels:
    • Story Points (Obsolete):
      40
    • Sprint:
      BACKEND Sprint 9

      Description

      Description from MDLQA test.

      For this test you'll at least one course with at least 1 grade item and at least 1 student. The student needs to have a grade for the grade item.

      While not necessary it is recommended that you have two browsers (ie Firefox and Chrome) so that you can be logged in as a student and a teacher simultaneously)

      1. As a teacher view the grader report. Check that the grade item(s) and the student's grade(s) are correct. Make a note of the student's grade(s).
      2. As the student go into the course. Under Course administration clicks on Grades. You may need to select the User report from the navigation drop down.
      3. Verify that the student's grade(s) are report correctly.
      4. Select the Overview report. Check that the student's course grade is correct.
      5. As the teacher click on Grades under Course Administration. Select the user report from the navigation drop down.
      6. Verify that the teacher can access any users User report via the second navigation drop down.
      7. Verify that there is an "All users" option in the user selection drop down. This should display all student user reports on a single page.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Mark Nelson added a comment -

            Hey [~davmon], I know you are always busy (because you are a super Catalan legend), but it would be awesome if you could review this for me. I have added some new step definitions for assigning and viewing grades in the gradebook and think it would be best for someone with your knowledge in the area to review. Cheers!

            Show
            Mark Nelson added a comment - Hey [~davmon] , I know you are always busy (because you are a super Catalan legend), but it would be awesome if you could review this for me. I have added some new step definitions for assigning and viewing grades in the gradebook and think it would be best for someone with your knowledge in the area to review. Cheers!
            Hide
            Andrew Davis added a comment -

            Whoops, removing myself as peer reviewer to leave it for David.

            Show
            Andrew Davis added a comment - Whoops, removing myself as peer reviewer to leave it for David.
            Hide
            David Monllaó added a comment -

            Sure Marco, np, I will review it today or tomorrow. Thanks

            Show
            David Monllaó added a comment - Sure Marco, np, I will review it today or tomorrow. Thanks
            Hide
            David Monllaó added a comment -

            Thanks for working on this Mark, the patch looks very good, only a couple of comments:

            Show
            David Monllaó added a comment - Thanks for working on this Mark, the patch looks very good, only a couple of comments: More info in the commit message about what is being tested As far as I see the component should be core_grades in plural Those acceptance tests are black boxed ( http://en.wikipedia.org/wiki/Black-box_testing ) and using database calls when it is not strictly necessary is cheating; most of the time there are ways to avoid using db calls, in fact I only had to use them once; you should inspect the DOM looking for a proper way to point to the nodes you want to interact with; examples: behat_grade::i_give_the_grade() && behat_grade::i_should_see_the_grade() -> The input [@type=text] box where you set the grade has a label which text you can use to point to it. Only the first line of a step definition will be seen by the test writer when listing the steps. http://docs.moodle.org/dev/Acceptance_testing#Example class behat_grade doesn't have a doc block If there is a single scenario there is no need to create a background section https://github.com/markn86/moodle/compare/master...MDL-43332_master#diff-0eeb27bc9e1e53a9e842eea24b211a6eR45 and other references. select [@name='jump'] can be referenced by it's label if I am not missing anything, you can *I select "User report" from "Grade report" https://github.com/markn86/moodle/compare/master...MDL-43332_master#diff-0eeb27bc9e1e53a9e842eea24b211a6eR49 same point above but with select [@name='userid'] , you can use the label Up to you as I'm being picky here https://github.com/markn86/moodle/compare/master...MDL-43332_master#diff-0eeb27bc9e1e53a9e842eea24b211a6eR44 I press "Update" would be more verbose, feel free to ignore me. Just in case, I would enrol a second student and check "All users (2)" but not sure if it is worth, up to you again.
            Hide
            Mark Nelson added a comment -

            Thanks David,

            1. Done.
            2. Hmm, the folder is called 'grade'. I did "git log --grep="core_grade:"" and it seems that is what is used in various places.
            3. For assigning a grade the label contains the user's fullname, so I still need to perform a DB query to retrieve this (same happens in the behat_grade.php). However, for viewing the grade I don't see how I can identify the table column without querying the DB for the item number. Can you advise?
            4. Ok, I have moved the comment into one line.
            5. There is no need to include a doc block for the class if it is the only thing in the file (see http://docs.moodle.org/dev/Coding_style#Classes_3).
            6. I put this in the background section in case another scenario was added to expand on this but have moved it as you have requested. I guess if some one else wants to add to this they can move it into the background section.
            7. Done.
            8. Done.
            9. Done.
            10. I don't find this necessary for this test.

            If you could review again and please comment how to achieve the viewing of the grade w/o the DB query that would be great!

            Cheers!

            Show
            Mark Nelson added a comment - Thanks David, Done. Hmm, the folder is called 'grade'. I did "git log --grep="core_grade:"" and it seems that is what is used in various places. For assigning a grade the label contains the user's fullname, so I still need to perform a DB query to retrieve this (same happens in the behat_grade.php). However, for viewing the grade I don't see how I can identify the table column without querying the DB for the item number. Can you advise? Ok, I have moved the comment into one line. There is no need to include a doc block for the class if it is the only thing in the file (see http://docs.moodle.org/dev/Coding_style#Classes_3 ). I put this in the background section in case another scenario was added to expand on this but have moved it as you have requested. I guess if some one else wants to add to this they can move it into the background section. Done. Done. Done. I don't find this necessary for this test. If you could review again and please comment how to achieve the viewing of the grade w/o the DB query that would be great! Cheers!
            Hide
            Michael de Raadt added a comment -

            Carrying over to the next sprint.

            Show
            Michael de Raadt added a comment - Carrying over to the next sprint.
            Hide
            David Monllaó added a comment -

            Thanks Mark,

            #2 In core_component::fetch_subsystems() I see core_grades (lib/classes/component.php) as far as I know this is the correct source of component names but please confirm it
            #3 In the given section you specified the system users data, including firstname and lastname, so you are not restricted to point to a cell using the user username, it will not be as flexible as fullname(), but in a scenario you have full control of all the site contents and configurations so it would not be a problem to not use fullname(). Code #1 pasted below.
            #5 Oh good thanks, I didn't know
            #6 Sounds good, the one adding another scenario will know where he/she needs to split it
            #10 It is not part of the test, I was mentioning it because we can possibly catch issues having a second user that we would not catch using just one, but it is fine as it is, you are right.

            I haven't tried the code but more or less something like this should work:

            // You would need to provide info about $userfullname in the step description
            public function i_give_the_grade($grade, $userfullname, $itemname) {
                $fieldstr = $userfullname . ' ' . $itemname . ' ' . get_string('gradestringid', 'gradestringcomponent');
             
                // You can point to the field using it's label.
                // We need to $this->escape() as $userfullname may contain double quotes.
                return Given('I fill in "' . $this->escape($fieldstr) . '" with "$grade"');
            }
            

            // You don't need the user as you are logged in as the user or you already selected the user from the users dropdown menu (if you would need it you can also use the firstname + lastname)
            public function i_should_see_the_grade($grade, $itemname) {
                return Given('I should see "$grade" in the "' . $this->escape($itemname) . '" "table_row"');
            }
            

            Show
            David Monllaó added a comment - Thanks Mark, #2 In core_component::fetch_subsystems() I see core_grades (lib/classes/component.php) as far as I know this is the correct source of component names but please confirm it #3 In the given section you specified the system users data, including firstname and lastname, so you are not restricted to point to a cell using the user username, it will not be as flexible as fullname(), but in a scenario you have full control of all the site contents and configurations so it would not be a problem to not use fullname(). Code #1 pasted below. #5 Oh good thanks, I didn't know #6 Sounds good, the one adding another scenario will know where he/she needs to split it #10 It is not part of the test, I was mentioning it because we can possibly catch issues having a second user that we would not catch using just one, but it is fine as it is, you are right. I haven't tried the code but more or less something like this should work: // You would need to provide info about $userfullname in the step description public function i_give_the_grade($grade, $userfullname, $itemname) { $fieldstr = $userfullname . ' ' . $itemname . ' ' . get_string('gradestringid', 'gradestringcomponent');   // You can point to the field using it's label. // We need to $this->escape() as $userfullname may contain double quotes. return Given('I fill in "' . $this->escape($fieldstr) . '" with "$grade"'); } // You don't need the user as you are logged in as the user or you already selected the user from the users dropdown menu (if you would need it you can also use the firstname + lastname) public function i_should_see_the_grade($grade, $itemname) { return Given('I should see "$grade" in the "' . $this->escape($itemname) . '" "table_row"'); }
            Hide
            Mark Nelson added a comment -

            Thanks for sharing your wisdom David 'the Behat legend' Monllao. I was looking at behat_groups.php and noticed that only the username is passed to the function, and then a DB called is made to retrieve the fullname, so assumed that was the correct way to do this. I am now submitting this for integration. Cheers!

            Show
            Mark Nelson added a comment - Thanks for sharing your wisdom David 'the Behat legend' Monllao. I was looking at behat_groups.php and noticed that only the username is passed to the function, and then a DB called is made to retrieve the fullname, so assumed that was the correct way to do this. I am now submitting this for integration. Cheers!
            Hide
            Mark Nelson added a comment -

            FYI, I ended up removing the last step definition as it could easily be written in the feature file in one line.

            Show
            Mark Nelson added a comment - FYI, I ended up removing the last step definition as it could easily be written in the feature file in one line.
            Hide
            Mark Nelson added a comment -

            Oh, also, yes I changed the tag to @core_grades. Moodle can be pretty inconsistent, why is this a plural? Oh well ..

            Show
            Mark Nelson added a comment - Oh, also, yes I changed the tag to @core_grades. Moodle can be pretty inconsistent, why is this a plural? Oh well ..
            Hide
            David Monllaó added a comment -

            np Lancelin bad ass

            About behat_group.php true true true, I was thinking on behat_cohort::i_add_user_to_cohort, where we don't have any other option than querying the db, but in behat_group.php we can use the firstname and the lastname; I've created MDL-43584

            Show
            David Monllaó added a comment - np Lancelin bad ass About behat_group.php true true true, I was thinking on behat_cohort::i_add_user_to_cohort, where we don't have any other option than querying the db, but in behat_group.php we can use the firstname and the lastname; I've created MDL-43584
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Uhm... as far as this is blocking MDL-31597 that is a real bug in stables... is there anything against backporting this also to 25 and 26 in order to get the fix tested there too?

            Show
            Eloy Lafuente (stronk7) added a comment - Uhm... as far as this is blocking MDL-31597 that is a real bug in stables... is there anything against backporting this also to 25 and 26 in order to get the fix tested there too?
            Hide
            Mark Nelson added a comment -

            Hmm, the only reason why I can see this blocking that issue is because they both create a grade/tests/behat/grade_view.feature. Any ways, I will backport this and test to make sure it works on 2.6 and 2.5.

            Show
            Mark Nelson added a comment - Hmm, the only reason why I can see this blocking that issue is because they both create a grade/tests/behat/grade_view.feature. Any ways, I will backport this and test to make sure it works on 2.6 and 2.5.
            Hide
            Mark Nelson added a comment -

            Backported and tested in 2.5 and 2.6 and all passes, so feel free to backport.

            Show
            Mark Nelson added a comment - Backported and tested in 2.5 and 2.6 and all passes, so feel free to backport.
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Great, "american legend", LOL!

            Show
            Eloy Lafuente (stronk7) added a comment - Great, "american legend", LOL!
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Integrated (25, 26 and master), thanks!

            Show
            Eloy Lafuente (stronk7) added a comment - Integrated (25, 26 and master), thanks!
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Passing for all branches with:

            1 escenario (1 exitosos)
            52 pasos (52 exitosos)
            0m52.756s
            

            Show
            Eloy Lafuente (stronk7) added a comment - Passing for all branches with: 1 escenario (1 exitosos) 52 pasos (52 exitosos) 0m52.756s
            Hide
            Eloy Lafuente (stronk7) added a comment -

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

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

            Well done is better than well said.

            Closing, big thanks!

            Show
            Eloy Lafuente (stronk7) added a comment - Well done is better than well said. Closing, big thanks!

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Agile