Moodle
  1. Moodle
  2. MDL-44108

Behat - add a data generator to create roles - "the following "roles" exists"

    Details

    • Rank:
      56367

      Description

      As commented here (https://moodle.org/mod/forum/discuss.php?d=254081) many 3rd parties writing their own tests may be interested in adding their custom roles.

      They also have the option to add a my custom local data set is loaded step definition.

        Activity

        Hide
        David Monllaó added a comment - - edited

        (To add to http://docs.moodle.org/dev/Acceptance_testing#Available_elements once this gets integrated)

        • Course categories
          • The required field is shortname
          • References the archetype role using the role shortname
            Given the following "roles" exists:
            name shortname description archetype
            Custom teacher role1 The coolest teacher editingteacher
            Role 2 name role2 My role 2 student
            (defaults to shortname) visitor (defaults to shortname)  
        Show
        David Monllaó added a comment - - edited (To add to http://docs.moodle.org/dev/Acceptance_testing#Available_elements once this gets integrated) Course categories The required field is shortname References the archetype role using the role shortname Given the following "roles" exists: name shortname description archetype Custom teacher role1 The coolest teacher editingteacher Role 2 name role2 My role 2 student (defaults to shortname) visitor (defaults to shortname)  
        Hide
        David Monllaó added a comment -

        I will backport it to 2.6 and 2.5 once reviewed.

        Show
        David Monllaó added a comment - I will backport it to 2.6 and 2.5 once reviewed.
        Hide
        CiBoT added a comment -

        Results for MDL-44108

        • Remote repository: git://github.com/dmonllao/moodle.git
        Show
        CiBoT added a comment - Results for MDL-44108 Remote repository: git://github.com/dmonllao/moodle.git Remote branch MDL-44108 _master to be integrated into upstream master Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/1330 Details: http://integration.moodle.org/job/Precheck%20remote%20branch/1330/artifact/work/smurf.html
        Hide
        Frédéric Massart added a comment -

        Hi David,

        Your code looks spot on, I did not find anything questionnable. There is a just tiny thing... the exception message when the archetype is not found will result in '[...] specific a validarchetype shortname [...]'. valid and archetype are glued.

        One thing that I would like you to comment on this issue is the reason why the datagenerator is not added to the core data generators. I can see that all the logic in process_role() is likely to be useful in PHP Unit tests, so to me it sounds like it could be in the main generator.

        Cheers,
        Fred

        Show
        Frédéric Massart added a comment - Hi David, Your code looks spot on, I did not find anything questionnable. There is a just tiny thing... the exception message when the archetype is not found will result in ' [...] specific a validarchetype shortname [...] '. valid and archetype are glued. One thing that I would like you to comment on this issue is the reason why the datagenerator is not added to the core data generators. I can see that all the logic in process_role() is likely to be useful in PHP Unit tests, so to me it sounds like it could be in the main generator. Cheers, Fred
        Hide
        David Monllaó added a comment -

        Thanks Fred, I've generalized the generator to testing/generator/data_generator.php as proposed. Sending to integration.

        Show
        David Monllaó added a comment - Thanks Fred, I've generalized the generator to testing/generator/data_generator.php as proposed. 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
        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 -

        Discussed with David - this seems good - but we will need a way to specify the capability changes to the custom roles to make this really useful. He will add a follow up issue for something like "Given the following role overrides exists:" (Unsure about the word overrides there).

        Show
        Damyon Wiese added a comment - Discussed with David - this seems good - but we will need a way to specify the capability changes to the custom roles to make this really useful. He will add a follow up issue for something like "Given the following role overrides exists:" (Unsure about the word overrides there).
        Hide
        David Monllaó added a comment -

        Damyon, just checked the existing capabilities, we already have permission overrides http://docs.moodle.org/dev/Acceptance_testing#Available_elements, sorry I didn't remember

        Show
        David Monllaó added a comment - Damyon, just checked the existing capabilities, we already have permission overrides http://docs.moodle.org/dev/Acceptance_testing#Available_elements , sorry I didn't remember
        Hide
        Damyon Wiese added a comment -

        Thanks David - all good then. Integrated to master only.

        Cheers!

        Show
        Damyon Wiese added a comment - Thanks David - all good then. Integrated to master only. Cheers!
        Hide
        David Monllaó added a comment -

        what I could do if you think we need it is to add a few more steps to ensure permission overrides works on custom roles

        Show
        David Monllaó added a comment - what I could do if you think we need it is to add a few more steps to ensure permission overrides works on custom roles
        Hide
        Damyon Wiese added a comment -

        If you want to add more tests I will be more than happy to integrate them (either in this issue or a new one)!

        Show
        Damyon Wiese added a comment - If you want to add more tests I will be more than happy to integrate them (either in this issue or a new one)!
        Hide
        David Monllaó added a comment -

        sure, np

        Show
        David Monllaó added a comment - sure, np
        Hide
        David Monllaó added a comment -

        On top of this one:
        git pull MDL-44108_master-custom-role-override

        Show
        David Monllaó added a comment - On top of this one: git pull MDL-44108 _master-custom-role-override
        Hide
        Rajesh Taneja added a comment -

        Thanks for fixing this David,

        Behat passed on nightly. Passing...

        Show
        Rajesh Taneja added a comment - Thanks for fixing this David, Behat passed on nightly. Passing...
        Hide
        Damyon Wiese added a comment -

        Parallel programmParallel programming is harding is hard

        Thanks for reporting/fixing/testing/improving Moodle. This issue has now been released.

        Show
        Damyon Wiese added a comment - Parallel programmParallel programming is harding is hard Thanks for reporting/fixing/testing/improving Moodle. This issue has now been released.
        Hide
        David Monllaó added a comment -

        http://docs.moodle.org/dev/Acceptance_testing#Available_elements updated, removing dev_docs_required label

        Show
        David Monllaó added a comment - http://docs.moodle.org/dev/Acceptance_testing#Available_elements updated, removing dev_docs_required label

          People

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

            Dates

            • Created:
              Updated:
              Resolved: