• No random tests.

  • Tests should use assert_equals.

  • Needs random tests

  • It's been a while, but I've updated the tests to catch invalid tests -- most notably, cyclic roles.

    Feel free to provide more feedback!

  • No test cases! Also semantics not fully specified (what if it has :apple and "apple" ?)

  • The tests should be augmented. The current tests don't seem to test (judging by accepted solutions) for the case when roles are not added implicitly to the role on which we call all_permissions, but only as sub-roles of other roles. E.g.:

    can_edit = Permission.new 'Edit'
    editors = Role.new 'Editor'
    editors.permissions << can_edit
    
    can_create = Permissions.new 'Create'
    creators = Role.new 'Creator'
    creators.permissions << can_create
    creators.roles << editors
    
    admins = Role.new 'Admin'
    admins.roles << creators # Note that only creators are added explicitly, by editors is the sub-role of creators
    
    Test.expect(admins.all_permissions.includes?(can_edit)) # Should include 'Edit' since 'Editors' are included in 'Creators'
    

    Also, should test for circular references announces in the kata description ("A role can reference itself"). E.g., extending my example from above:

    can_edit = Permission.new 'Edit'
    editors = Role.new 'Editor'
    editors.permissions << can_edit
    
    can_create = Permissions.new 'Create'
    creators = Role.new 'Creator'
    creators.permissions << can_create
    creators.roles << editors
    
    admins = Role.new 'Admin'
    admins.roles << creators
    admins.roles << admins # circular reference!
    
    Test.assert_equals( admins.all_permissions.map(&:name).sort, ['Create','Edit'] ) # Still should include the two permissions
    
  • Whew, that was a great, challenging kata - nice job!

  • You should test for permissions inherited from two levels up; there's currently at least one solution that doesn't support them.

  • Thanks for the feedback!

    Originally I had intended only to override the hsh[:key] methods. Now that you mention it, I might consider testing for the other methods you've mentioned as well :P

  • This comment is hidden because it contains spoiler information about the solution