Details

    • Type: Task Task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.6
    • Fix Version/s: 2.6.3
    • Component/s: Assignment
    • Labels:
    • Testing Instructions:
      Hide

      Ghostscript needs to be set up for the editpdf plugin (on ubuntu ghostscript is installed by default, so nothing need to be setup).

      vendor/bin/behat --config=/home/jerome/moodles/stable_master_hq/moodledata_behat/behat/behat.yml --name "Submit a PDF file as a student and annotate the PDF as a teacher"

      Test this with phantomjs and make sure this should skip.

      Show
      Ghostscript needs to be set up for the editpdf plugin (on ubuntu ghostscript is installed by default, so nothing need to be setup). vendor/bin/behat --config=/home/jerome/moodles/stable_master_hq/moodledata_behat/behat/behat.yml --name "Submit a PDF file as a student and annotate the PDF as a teacher" Test this with phantomjs and make sure this should skip.
    • Affected Branches:
      MOODLE_26_STABLE
    • Fixed Branches:
      MOODLE_26_STABLE
    • Pull 2.6 Branch:
    • Pull Master Branch:
      MDL-42025-master-nomerge
    • Story Points (Obsolete):
      20
    • Sprint:
      FRONTEND Sprint 10

      Description

      We need to add behat tests for the Edit PDF plugin - but there are 2 things that require solving first.

      1. How to simulate mouse/touch drags
      2. How to skip the tests if ghostscript is not available.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Jérôme Mouneyrac added a comment -

            After talking with David we agreed that there were no need to implement complicated behat test for the drawing tool. So we'll be testing the basic function as opening the pdf, selecting the tool, testing ghostscript, adding stamp...

            Show
            Jérôme Mouneyrac added a comment - After talking with David we agreed that there were no need to implement complicated behat test for the drawing tool. So we'll be testing the basic function as opening the pdf, selecting the tool, testing ghostscript, adding stamp...
            Hide
            Jérôme Mouneyrac added a comment -

            I added a behat shortcut, you can now find the first filemanager of the page if you don't mention the filepicker label.

            Show
            Jérôme Mouneyrac added a comment - I added a behat shortcut, you can now find the first filemanager of the page if you don't mention the filepicker label.
            Hide
            Andrew Davis added a comment -

            This issue needs testing instructions.

            Are these steps correct? (the empty quotes)

            + And I upload "pix/help.png" file to "" filepicker
            + And I upload "pix/docs.png" file to "" filepicker

            Otherwise I think this looks good. Is there any value in having the student log back in to verify that the feedback document is available to them?

            Show
            Andrew Davis added a comment - This issue needs testing instructions. Are these steps correct? (the empty quotes) + And I upload "pix/help.png" file to "" filepicker + And I upload "pix/docs.png" file to "" filepicker Otherwise I think this looks good. Is there any value in having the student log back in to verify that the feedback document is available to them?
            Hide
            Jérôme Mouneyrac added a comment -

            Thanks Andrew, they are correct against the tweak I added to the step. Adding the test instruction and sending to integration.

            Show
            Jérôme Mouneyrac added a comment - Thanks Andrew, they are correct against the tweak I added to the step. Adding the test instruction and sending to integration.
            Hide
            Eloy Lafuente (stronk7) added a comment -

            The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

            TIA and ciao

            Show
            Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
            Hide
            Damyon Wiese added a comment -

            Thanks Jerome - but this behat test is failing.

            Field matching locator "'assignfeedback_editpdf_enabled'" not found.

            This has been removed and the test needs updating.

            Please resubmit once the test is passing!

            Show
            Damyon Wiese added a comment - Thanks Jerome - but this behat test is failing. Field matching locator "'assignfeedback_editpdf_enabled'" not found. This has been removed and the test needs updating. Please resubmit once the test is passing!
            Hide
            CiBoT added a comment -

            Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

            Show
            CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
            Hide
            Jérôme Mouneyrac added a comment -

            Fix done, resubmitting.

            Show
            Jérôme Mouneyrac added a comment - Fix done, resubmitting.
            Hide
            Damyon Wiese added a comment -

            Thanks Jerome, but looking at the patch, you do not seem to have implemented part 2 (skip the test if ghostscript is not available).

            Ghostscript is an optional requirement, but this will produce behat failures on all sites that do not have it installed (like our CI server).

            Lets leave this until after the release (tests get free backport votes) and come up with a nice way to do the skipping of the test.

            Show
            Damyon Wiese added a comment - Thanks Jerome, but looking at the patch, you do not seem to have implemented part 2 (skip the test if ghostscript is not available). Ghostscript is an optional requirement, but this will produce behat failures on all sites that do not have it installed (like our CI server). Lets leave this until after the release (tests get free backport votes) and come up with a nice way to do the skipping of the test.
            Hide
            CiBoT added a comment -

            Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

            Show
            CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
            Hide
            Jérôme Mouneyrac added a comment - - edited

            Behat conditional testing needs to be implemented, I created: MDL-43330
            The issue is also blocked by a behat bug: MDL-43266

            Show
            Jérôme Mouneyrac added a comment - - edited Behat conditional testing needs to be implemented, I created: MDL-43330 The issue is also blocked by a behat bug: MDL-43266
            Hide
            Jérôme Mouneyrac added a comment -

            Sent to peer-review.
            PS: currently blocked per two issues (see above comment)

            Show
            Jérôme Mouneyrac added a comment - Sent to peer-review. PS: currently blocked per two issues (see above comment)
            Hide
            Jérôme Mouneyrac added a comment -

            This is issue can be peer-reviewed once both behat blockers are resolved. No need to peer-reviewed before as if the blockers change this issue could be changed.

            Show
            Jérôme Mouneyrac added a comment - This is issue can be peer-reviewed once both behat blockers are resolved. No need to peer-reviewed before as if the blockers change this issue could be changed.
            Hide
            Jérôme Mouneyrac added a comment -

            I'm sending it back to peer-review as I'm leaving in holidays. I rebased, and updated the code to the new SkippedException and fixed the newly deprecated filepicker behat step. So basically review it when both blocker as resolved and send it straight to integration. Hopefully all should go well

            Show
            Jérôme Mouneyrac added a comment - I'm sending it back to peer-review as I'm leaving in holidays. I rebased, and updated the code to the new SkippedException and fixed the newly deprecated filepicker behat step. So basically review it when both blocker as resolved and send it straight to integration. Hopefully all should go well
            Hide
            Andrew Davis added a comment -

            This issue is just waiting on the two links issues that it is blocked by. Once they are integrated this issue can go up for integration.

            Show
            Andrew Davis added a comment - This issue is just waiting on the two links issues that it is blocked by. Once they are integrated this issue can go up for integration.
            Hide
            CiBoT added a comment -

            Results for MDL-42025

            • Remote repository: git://github.com/mouneyrac/moodle.git
            Show
            CiBoT added a comment - Results for MDL-42025 Remote repository: git://github.com/mouneyrac/moodle.git Remote branch MDL-42025 -master-3 to be integrated into upstream master Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/366 Warning: The MDL-42025 -master-3 branch at git://github.com/mouneyrac/moodle.git has not been rebased recently. Details: http://integration.moodle.org/job/Precheck%20remote%20branch/366/artifact/work/smurf.html
            Hide
            Andrew Davis added a comment -

            This issue is still waiting for a linked issue. Jerome, if you haven't done it recently it would be worth rebasing given the amount of time this has been parked.

            Show
            Andrew Davis added a comment - This issue is still waiting for a linked issue. Jerome, if you haven't done it recently it would be worth rebasing given the amount of time this has been parked.
            Hide
            Jérôme Mouneyrac added a comment -

            I've just rebased and updated to the last changes in MDL-43266.

            Show
            Jérôme Mouneyrac added a comment - I've just rebased and updated to the last changes in MDL-43266 .
            Hide
            Andrew Davis added a comment -

            Sending this on to integration with the other issue.

            Show
            Andrew Davis added a comment - Sending this on to integration with the other issue.
            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 -

            Hi Jerome,
            this issue is pending the blocker yet but I had a quick look. First comment is camelCase is not allowed in moodle (in behat step definition). Second - I'm not sure I understand the point of changing the function get_filepicker_node() - what is the problem of specifying filepicker name in the behat test? Maybe [~davmon] can review this little bit please?
            Thanks

            Show
            Marina Glancy added a comment - Hi Jerome, this issue is pending the blocker yet but I had a quick look. First comment is camelCase is not allowed in moodle (in behat step definition). Second - I'm not sure I understand the point of changing the function get_filepicker_node() - what is the problem of specifying filepicker name in the behat test? Maybe [~davmon] can review this little bit please? Thanks
            Hide
            David Monllaó added a comment -

            Looks good Jerome, only a few comments/suggestions, all minor things:

            Show
            David Monllaó added a comment - Looks good Jerome, only a few comments/suggestions, all minor things: Would be good if testing instructions comments about the need to set ghostscript to run the scenario https://github.com/mouneyrac/moodle/compare/5ad2075...MDL-42025-master-4#diff-0112b7525db5c29023b8ca9615d21d8eR1 -> http://docs.moodle.org/dev/Acceptance_testing#Writing_features #3, the plugin type should also be included https://github.com/mouneyrac/moodle/compare/5ad2075...MDL-42025-master-4#diff-0112b7525db5c29023b8ca9615d21d8eR21 I don't see the point of login as teacher to create the activity and then switch to admin https://github.com/mouneyrac/moodle/compare/5ad2075...MDL-42025-master-4#diff-0112b7525db5c29023b8ca9615d21d8eR61 you could probably use css_element to avoid xpath_element (which always looks scary to people) you would have things like .stampbutton, up to you https://github.com/mouneyrac/moodle/compare/5ad2075...MDL-42025-master-4#diff-fba23bd8c9e748fd10ca2342574e0c7dR71 maybe you can use another exception message to provide more accurate info If you feel like you want to refine how the scenario looks you can remove unnecessary Then and replace them by And for readability, as commented in many issues before, this would not be a justification to reject a patch. ( http://dannorth.net/whats-in-a-story/ ) https://github.com/mouneyrac/moodle/compare/5ad2075...MDL-42025-master-4#diff-0112b7525db5c29023b8ca9615d21d8eR42 https://github.com/mouneyrac/moodle/compare/5ad2075...MDL-42025-master-4#diff-0112b7525db5c29023b8ca9615d21d8eR51 https://github.com/mouneyrac/moodle/compare/5ad2075...MDL-42025-master-4#diff-0112b7525db5c29023b8ca9615d21d8eR68 https://github.com/mouneyrac/moodle/compare/5ad2075...MDL-42025-master-4#diff-fba23bd8c9e748fd10ca2342574e0c7dR66 Should begin with uppercase
            Hide
            Sam Hemelryk added a comment -

            Reopening this issue this week sorry Jerome - there are points from Marina and David to consider and the blocking MDL-43266 has been reopened as well.

            Show
            Sam Hemelryk added a comment - Reopening this issue this week sorry Jerome - there are points from Marina and David to consider and the blocking MDL-43266 has been reopened as well.
            Hide
            CiBoT added a comment -

            Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

            Show
            CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
            Hide
            Jérôme Mouneyrac added a comment - - edited

            Camelcase: I think I respected the behat way and how it is done in other place in Moodle.
            Changing the function get_filepicker_node(): because behat can not find the filepicker in the admin pages. It is not supported. So my change is a little trick that could be useful in some other similar case too.

            As for David's review, the issue landed to integration before his review. Fixing them.

            Show
            Jérôme Mouneyrac added a comment - - edited Camelcase: I think I respected the behat way and how it is done in other place in Moodle. Changing the function get_filepicker_node(): because behat can not find the filepicker in the admin pages. It is not supported. So my change is a little trick that could be useful in some other similar case too. As for David's review, the issue landed to integration before his review. Fixing them.
            Hide
            Jérôme Mouneyrac added a comment -

            I fixed all except:
            https://github.com/mouneyrac/moodle/compare/5ad2075...MDL-42025-master-4#diff-fba23bd8c9e748fd10ca2342574e0c7dR71 maybe you can use another exception message to provide more accurate info => it's the same exception than on the other part of the code (keeping returned exception consistent).

            Sending the issue back to integration.

            Show
            Jérôme Mouneyrac added a comment - I fixed all except: https://github.com/mouneyrac/moodle/compare/5ad2075...MDL-42025-master-4#diff-fba23bd8c9e748fd10ca2342574e0c7dR71 maybe you can use another exception message to provide more accurate info => it's the same exception than on the other part of the code (keeping returned exception consistent). Sending the issue back to integration.
            Hide
            Jérôme Mouneyrac added a comment -

            Retested on 2.6/master.

            Show
            Jérôme Mouneyrac added a comment - Retested on 2.6/master.
            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
            Eloy Lafuente (stronk7) added a comment -

            Hi Jerome,

            I'm reopening this, needs minor tidying...

            1) The method name, I really think it must follow Moodle rules (ghostscript_is_installed), not Behat rules. It seems your ghostscriptIsInstalled() is the only one using CamelCase:

            grep -r 'function ' | grep 'tests/behat'
            

            2) There are some minor code checker things to fix (ignore the phpdocs ones related to unit tests.

            http://integration.moodle.org/job/Precheck%20remote%20branch/1240/artifact/work/smurf.html

            3) I think something is not completely correct in the ghostscriptIsInstalled(). A site with zlib installed and ghostscript missing won't be skipping the test but will fail (perhaps that "and" should be an "or" or so). More yet, where is that Moodle\BehatExtension\Exception\SkippedException available? It's throwing here a nasty "Class not found" error.

            4) I'm not sure about this, have tried to find a similar case but failed, please confirm it with David. I think that, at some point, we agreed that all the subplugins should also include their "parent" tags, so in this case both "@mod" and "@mod_assing" should be present. That way, when those tags are selected for a test run, all the subplugins will, automatically, be included. Again, I've failed to find an example/documentation. Plz. confirm it there.

            Ciao

            Show
            Eloy Lafuente (stronk7) added a comment - Hi Jerome, I'm reopening this, needs minor tidying... 1) The method name, I really think it must follow Moodle rules (ghostscript_is_installed), not Behat rules. It seems your ghostscriptIsInstalled() is the only one using CamelCase: grep -r 'function ' | grep 'tests/behat' 2) There are some minor code checker things to fix (ignore the phpdocs ones related to unit tests. http://integration.moodle.org/job/Precheck%20remote%20branch/1240/artifact/work/smurf.html 3) I think something is not completely correct in the ghostscriptIsInstalled(). A site with zlib installed and ghostscript missing won't be skipping the test but will fail (perhaps that "and" should be an "or" or so). More yet, where is that Moodle\BehatExtension\Exception\SkippedException available? It's throwing here a nasty "Class not found" error. 4) I'm not sure about this, have tried to find a similar case but failed, please confirm it with David. I think that, at some point, we agreed that all the subplugins should also include their "parent" tags, so in this case both "@mod" and "@mod_assing" should be present. That way, when those tags are selected for a test run, all the subplugins will, automatically, be included. Again, I've failed to find an example/documentation. Plz. confirm it there. Ciao
            Hide
            CiBoT added a comment -

            Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

            Show
            CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
            Hide
            Jérôme Mouneyrac added a comment - - edited

            Thanks Eloy, I fixed all your points (except the @Given tag not recognized by the smurf). David agreed that for 4) @mod and @mod_assign make sense. Retesting when ghostscript is uninstalled and submitting.

            Show
            Jérôme Mouneyrac added a comment - - edited Thanks Eloy, I fixed all your points (except the @Given tag not recognized by the smurf). David agreed that for 4) @mod and @mod_assign make sense. Retesting when ghostscript is uninstalled and submitting.
            Hide
            Jérôme Mouneyrac added a comment -

            I have trouble to reproduce "Class not found". Moodle\BehatExtension\Exception\SkippedException do generate the skip test.

            I fixed the rest of the mentioned problemes. I'll send this issue back to integration but it is blocked by MDL-43266.

            Show
            Jérôme Mouneyrac added a comment - I have trouble to reproduce "Class not found". Moodle\BehatExtension\Exception\SkippedException do generate the skip test. I fixed the rest of the mentioned problemes. I'll send this issue back to integration but it is blocked by MDL-43266 .
            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
            Damyon Wiese added a comment -

            This issue is blocked by an issue that is not up for integration and will not be reviewed this week - sorry there's lots in the queue.

            Show
            Damyon Wiese added a comment - This issue is blocked by an issue that is not up for integration and will not be reviewed this week - sorry there's lots in the queue.
            Hide
            Eloy Lafuente (stronk7) added a comment -

            The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

            TIA and ciao

            Show
            Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
            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
            Damyon Wiese added a comment -

            This issue is "still" blocked by an issue that is not in integration. Reopening.

            Show
            Damyon Wiese added a comment - This issue is "still" blocked by an issue that is not in integration. Reopening.
            Hide
            CiBoT added a comment -

            Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

            Show
            CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
            Hide
            Jérôme Mouneyrac added a comment -

            I sent the other one to integration, sending back this one then

            Show
            Jérôme Mouneyrac added a comment - I sent the other one to integration, sending back this one then
            Hide
            Jérôme Mouneyrac added a comment -

            reopening as I reopened the blocker...

            Show
            Jérôme Mouneyrac added a comment - reopening as I reopened the blocker...
            Hide
            Jérôme Mouneyrac added a comment -

            resubmitting

            Show
            Jérôme Mouneyrac added a comment - resubmitting
            Hide
            Eloy Lafuente (stronk7) added a comment -

            The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

            TIA and ciao

            Show
            Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
            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
            Sam Hemelryk added a comment -

            Ah - blocked by MDL-43266 - stopping my review pending that issue.

            Show
            Sam Hemelryk added a comment - Ah - blocked by MDL-43266 - stopping my review pending that issue.
            Show
            David Monllaó added a comment - After MDL-43738 and MDL-43236 https://github.com/mouneyrac/moodle/compare/cc74843...MDL-42025-master-nomerge#diff-2 needs to be updated
            Hide
            Eloy Lafuente (stronk7) added a comment -

            David, can you clarify your previous comment? It seems that all the related issues (MDL-43738, MDL-43236 and MDL-43266) have been already integrated. Then... what's needed here?

            Show
            Eloy Lafuente (stronk7) added a comment - David, can you clarify your previous comment? It seems that all the related issues ( MDL-43738 , MDL-43236 and MDL-43266 ) have been already integrated. Then... what's needed here?
            Hide
            David Monllaó added a comment -

            Sure: That issues are deprecating step definitions in master and the scenarios needs to be updated according to them, I will attach a patch on top of it updating it

            Show
            David Monllaó added a comment - Sure: That issues are deprecating step definitions in master and the scenarios needs to be updated according to them, I will attach a patch on top of it updating it
            Hide
            David Monllaó added a comment -

            In case you are keen to integrate it in this cycle, I've updated the master pull branch with my own branch (on top of all Jerome's commits) replacing the deprecated steps for the new ones.

            Note that I've manually fixed the MDL-43266 conflict so there is no need to pull https://github.com/dmonllao/moodle/commit/5e4dd1b35c27884cf4934a0835ff5fc5d0dc9032 patch and only pull FETCH_HEAD~4..FETCH_HEAD

            Show
            David Monllaó added a comment - In case you are keen to integrate it in this cycle, I've updated the master pull branch with my own branch (on top of all Jerome's commits) replacing the deprecated steps for the new ones. Note that I've manually fixed the MDL-43266 conflict so there is no need to pull https://github.com/dmonllao/moodle/commit/5e4dd1b35c27884cf4934a0835ff5fc5d0dc9032 patch and only pull FETCH_HEAD~4..FETCH_HEAD
            Hide
            Eloy Lafuente (stronk7) added a comment -

            The integration of this issue has been delayed to next week because the integration period is over (Monday, Tuesday) and testing must happen on Wednesday.

            This change to a more rigid timeframe on each integration/testing cycle aims to produce a better and clear separation and organization of tasks for everybody.

            This is a bulk-automated message, so if you want to blame somebody/thing/where, don't do it here (use git instead) :-D :-P

            Apologies for the inconvenience, this will be integrated next week. Thanks for your collaboration & ciao

            Show
            Eloy Lafuente (stronk7) added a comment - The integration of this issue has been delayed to next week because the integration period is over (Monday, Tuesday) and testing must happen on Wednesday. This change to a more rigid timeframe on each integration/testing cycle aims to produce a better and clear separation and organization of tasks for everybody. This is a bulk-automated message, so if you want to blame somebody/thing/where, don't do it here (use git instead) :-D :-P Apologies for the inconvenience, this will be integrated next week. Thanks for your collaboration & ciao
            Hide
            Eloy Lafuente (stronk7) added a comment -

            The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

            TIA and ciao

            Show
            Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
            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 -

            ok, this is integrated in 2.6 and master.

            There was some mess in branches, the file lib/testing/tests/util_test.php was changed in both 2.6 and master without any need for it (just coding style changes) and it conflicted when merging. Those changes were discarded.

            Also the branch indicated for master did not exist. The branch indicated in diff url has deprecated steps. So I took the branch MDL-42025-master-nomerge and added a commit on top replacing the deprecated steps.

            I would not usually do it myself but Jerome has left HQ so I sort of can't send this back.

            Show
            Marina Glancy added a comment - ok, this is integrated in 2.6 and master. There was some mess in branches, the file lib/testing/tests/util_test.php was changed in both 2.6 and master without any need for it (just coding style changes) and it conflicted when merging. Those changes were discarded. Also the branch indicated for master did not exist. The branch indicated in diff url has deprecated steps. So I took the branch MDL-42025 -master-nomerge and added a commit on top replacing the deprecated steps. I would not usually do it myself but Jerome has left HQ so I sort of can't send this back.
            Hide
            Rajesh Taneja added a comment -

            This is failing phantomjs on nightly.
            Failing this for now, will provide more information soon.

            Show
            Rajesh Taneja added a comment - This is failing phantomjs on nightly. Failing this for now, will provide more information soon.
            Hide
            Marina Glancy added a comment -

            right, there was a wrong behat tag @_only_local. Should be @_file_upload

            I corrected the tag, now the test will be excluded from phantomjs

            Show
            Marina Glancy added a comment - right, there was a wrong behat tag @_only_local. Should be @_file_upload I corrected the tag, now the test will be excluded from phantomjs
            Hide
            Rajesh Taneja added a comment - - edited

            Thanks Marina,

            Tested with Phantomjs and it's all good, test is excluded now.

            Show
            Rajesh Taneja added a comment - - edited Thanks Marina, Tested with Phantomjs and it's all good, test is excluded now.
            Hide
            Michael de Raadt added a comment -

            Test result: Success!

            Tested in 2.6 and master branches.

            Selenium tests pass nicely.

            Show
            Michael de Raadt added a comment - Test result: Success! Tested in 2.6 and master branches. Selenium tests pass nicely.
            Hide
            Marina Glancy added a comment -

            Thanks for your awesome code, it is now part of Moodle. Don't forget to submit another issue next week (and peer review two).

            Show
            Marina Glancy added a comment - Thanks for your awesome code, it is now part of Moodle. Don't forget to submit another issue next week (and peer review two).

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Agile