-
Notifications
You must be signed in to change notification settings - Fork 215
Issue 2525 #2534
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
Issue 2525 #2534
Conversation
* master: (37 commits) corrected tying mistake corrected uri for creating org added comment about data length removed unused import removed unused import added api asciidoc and removed unesecary post method added config documentation organization improvements added config script so feature can be automatically configured from outside for example docker etc pass proper config to provider changed log level if dsn manager is not configured provided alias config added more fields added create tests #2493 log response added enabled impl and added tests for create org #2493 check style fixes formatting organization dns operation crud mofifications added config and dns manager interface ... Conflicts: restcomm/restcomm.testsuite/src/test/resources/restcomm.script_accounts_test
* master: added domain name in api response and changed param to uri info to read from context
* master: removed incorrect entry
* master: added examples for organization api added hostedzone param removed unnec log testuite refactored to allow parallel execution fixes #2502
* master: We want this file to be executed last since its contains xmlstarlet based config script https://telestax.atlassian.net/browse/RESTCOMM-1140 Added WavUtils tests
* master: created profile for paralel testing to prevent errors running single test from IDEA
@@ -568,6 +583,79 @@ public void testPasswordStrengthForAccountUpdate() { | |||
assertEquals(200, response.getStatus()); | |||
} | |||
|
|||
@Test | |||
public void testCreateAccountInSpecificOrganization() { | |||
// child should not be able to create account in specified org that it does not belong to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is just a general comment: not sure why you group all this scenarios in one testcase (i think they sequence you are testing now is not related, just in theme), and put a comment. It may be different testcases with proper test intention as part of test case emthod name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
intention is to test both
- permission
- creation process
reason i would avoid a separate test case is performance, it will take time to gear up a new archive just to separately test above 2 scenarios
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure performance is a reason, since i was thinking in creating new test methods/cases inside same test class. For a single test class, there is single container, and deployment of restcomm...but again, this is general comment on testing practices
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right @jaimecasero agreed on practices, i have split them into separate cases
@@ -28,6 +28,10 @@ | |||
SELECT * FROM restcomm_accounts WHERE parent_sid=#{parent_sid}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is this mariadb script? is this something that leaked during testing?
im trying to switch all files generated during test into maven "target" folder, so its inmediately ignore by git, and we dont get into this situations. can we change where the db is started for tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jaimecasero both mariadb/mysql and hsql have slightly different syntax that is why we have 2 seperate query scripts.
in testsuit we use hsql scripts only
testsuit is clear for this PR.
@gvagenas @jaimecasero @ghjansen @knosach can anyone please review