Details

    • Type: Task
    • Status: Closed
    • Priority: 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 Master Branch:
      MDL-42025-master-nomerge
    • Sprint:
      FRONTEND Sprint 10
    • 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

          Attachments

            Issue Links

              Activity

              Hide
              jerome 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
              jerome 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
              jerome 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
              jerome 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
              andyjdavis 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
              andyjdavis 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
              jerome 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
              jerome 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
              stronk7 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
              stronk7 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 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 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 CiBoT added a comment -

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

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

              Fix done, resubmitting.

              Show
              jerome Jérôme Mouneyrac added a comment - Fix done, resubmitting.
              Hide
              damyon 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 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 CiBoT added a comment -

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

              Show
              cibot CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
              Hide
              jerome 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
              jerome 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
              jerome Jérôme Mouneyrac added a comment -

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

              Show
              jerome Jérôme Mouneyrac added a comment - Sent to peer-review. PS: currently blocked per two issues (see above comment)
              Hide
              jerome 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
              jerome 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
              jerome 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
              jerome 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
              andyjdavis 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
              andyjdavis 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 CiBoT added a comment -

              Results for MDL-42025

              • Remote repository: git://github.com/mouneyrac/moodle.git
              Show
              cibot 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
              andyjdavis 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
              andyjdavis 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
              jerome Jérôme Mouneyrac added a comment -

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

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

              Sending this on to integration with the other issue.

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

              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 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
              dmonllao David Monllaó added a comment -

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

              Show
              dmonllao 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
              samhemelryk 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
              samhemelryk 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 CiBoT added a comment -

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

              Show
              cibot CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
              Hide
              jerome 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
              jerome 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
              jerome 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
              jerome 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
              jerome Jérôme Mouneyrac added a comment -

              Retested on 2.6/master.

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

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

              Show
              cibot CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
              Hide
              jerome 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
              jerome 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
              jerome 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
              jerome 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 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
              damyon 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 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
              stronk7 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
              stronk7 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 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
              damyon Damyon Wiese added a comment -

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

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

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

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

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

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

              reopening as I reopened the blocker...

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

              resubmitting

              Show
              jerome Jérôme Mouneyrac added a comment - resubmitting
              Hide
              stronk7 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
              stronk7 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 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
              samhemelryk Sam Hemelryk added a comment -

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

              Show
              samhemelryk Sam Hemelryk added a comment - Ah - blocked by MDL-43266 - stopping my review pending that issue.
              Show
              dmonllao 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
              stronk7 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
              stronk7 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
              dmonllao 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
              dmonllao 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
              dmonllao 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
              dmonllao 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
              stronk7 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
              stronk7 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
              stronk7 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
              stronk7 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 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 -

              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 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
              rajeshtaneja Rajesh Taneja added a comment -

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

              Show
              rajeshtaneja Rajesh Taneja added a comment - This is failing phantomjs on nightly. Failing this for now, will provide more information soon.
              Hide
              marina 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 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
              rajeshtaneja Rajesh Taneja added a comment - - edited

              Thanks Marina,

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

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

              Test result: Success!

              Tested in 2.6 and master branches.

              Selenium tests pass nicely.

              Show
              salvetore Michael de Raadt added a comment - Test result: Success! Tested in 2.6 and master branches. Selenium tests pass nicely.
              Hide
              marina 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 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:
                    Fix Release Date:
                    12/May/14