Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added Unit Tests for addPolicies and removePolicies #66

Merged
merged 3 commits into from
Mar 6, 2020

Conversation

drholmie
Copy link
Contributor

@drholmie drholmie commented Mar 5, 2020

Fixes #65.
Added the following tests in management_api.rs:

  • test_modify_policies_api
  • test_modify_grouping_policies_api

For:

async fn add_policies(&mut self, paramss: Vec<Vec<&str>>) -> Result<bool>;
async fn remove_policies(&mut self, paramss: Vec<Vec<&str>>) -> Result<bool>;
async fn add_named_policies(&mut self, ptype: &str, paramss: Vec<Vec<&str>>) -> Result<bool>;
async fn remove_named_policies(&mut self, ptype: &str, paramss: Vec<Vec<&str>>)
        -> Result<bool>;
async fn add_grouping_policies(&mut self, paramss: Vec<Vec<&str>>) -> Result<bool>;
async fn remove_grouping_policies(&mut self, paramss: Vec<Vec<&str>>) -> Result<bool>;
async fn add_named_grouping_policies(
        &mut self,
        ptype: &str,
        paramss: Vec<Vec<&str>>,
    ) -> Result<bool>;
async fn remove_named_grouping_policies(
        &mut self,
        ptype: &str,
        paramss: Vec<Vec<&str>>,
    ) -> Result<bool>;

Modified the following tests in rbac_api.rs:

  • test_role_api
  • test_permission_api
  • test_role_api_threads

For:

async fn add_permissions_for_user(
        &mut self,
        user: &str,
        permissions: Vec<Vec<&str>>,
    ) -> Result<bool>;
async fn add_roles_for_user(
        &mut self,
        user: &str,
        roles: Vec<&str>,
        domain: Option<&str>,
    ) -> Result<bool>;

Fixes casbin#65.
Added the following tests in `management_api.rs`:
- `test_modify_policies_api`
- `test_modify_policies_api`

Modified the following tests in `rbac_api.rs`:
- `test_role_api`
- `test_permission_api`
- `test_role_api_threads`
@PsiACE
Copy link
Contributor

PsiACE commented Mar 5, 2020

Could you please format the code? It looks like the CI build failed.

@drholmie
Copy link
Contributor Author

drholmie commented Mar 5, 2020

Thank you for pointing that out! Formatted the code.

@codecov
Copy link

codecov bot commented Mar 5, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@36703bb). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master      #66   +/-   ##
=========================================
  Coverage          ?   80.46%           
=========================================
  Files             ?       18           
  Lines             ?     2580           
  Branches          ?        0           
=========================================
  Hits              ?     2076           
  Misses            ?      504           
  Partials          ?        0
Impacted Files Coverage Δ
src/rbac_api.rs 86.22% <100%> (ø)
src/management_api.rs 75.7% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 36703bb...109b9a3. Read the comment docs.

@hsluoyz
Copy link
Member

hsluoyz commented Mar 5, 2020

Please add rust after "```" to colorize your text block.

@GopherJ
Copy link
Member

GopherJ commented Mar 5, 2020

Hello @drholmie I believe there are many unwrap without assert, could you add them? Also thanks for your contributions.

@drholmie
Copy link
Contributor Author

drholmie commented Mar 6, 2020

Ah! Thank you for pointing that out @GopherJ I believe you meant in the management_api.rs file? I'll begin adding those too.

@drholmie
Copy link
Contributor Author

drholmie commented Mar 6, 2020

Added more assert, @GopherJ do let me know if more are required. Thank you again for going through the PR!

@GopherJ
Copy link
Member

GopherJ commented Mar 6, 2020

Thanks!

@GopherJ GopherJ merged commit e9c2260 into casbin:master Mar 6, 2020
@drholmie
Copy link
Contributor Author

Thanks for approving this and for all the help :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add unit tests for addPolicies and removePolicies
4 participants