Added users deletion at the end of tests#240
Conversation
|
|
||
| @AfterClass | ||
| public static void tearDownClass() { | ||
| System.out.println("Start TearDownClass"); |
There was a problem hiding this comment.
Maybe use logger or completely remove this?
There was a problem hiding this comment.
I added on purpose so we have better visibility if the teardown happened or not. This for trying and catch the instances where users are not being cleaned-up.
It needs to be sent to the Travis log and system.out.println is the way.
There was a problem hiding this comment.
TearDown must always happen, unless there was a complete disaster and the process just died.
I find this print completely redundant. you can check it in this branch, I would avoid committing it to master.
There was a problem hiding this comment.
This is the thing... it doesn't always happen. This is why we have leftovers that blocks the testing account.
I added this so the next time it'll happen, we can track if the tearDown happened or not.
| e.printStackTrace(); | ||
| } | ||
| } | ||
| System.out.println("### Deleted - SubAccounts:"+createdSubAccountIds.size()+", Users:"+createdUserIds.size()+ ", UserGroups:"+createdGroupIds.size()); |
There was a problem hiding this comment.
Maybe use logger or completely remove this?
There was a problem hiding this comment.
Same here... will help us track why it's sometimes not cleaning up
| assertTrue("User2 id should be in the result set", returnedIds.contains(id2)); | ||
| assertTrue("User1 id should be in the result set", returnedIds.contains(user1Id)); | ||
| assertTrue("User2 id should be in the result set", returnedIds.contains(user2Id)); | ||
| deleteUser(user1Id); |
There was a problem hiding this comment.
putting deleteUser before assertions would provide a better chance for it to happen, like you perform request, then delete user, then assert results
There was a problem hiding this comment.
What has better chances to happen?
There was a problem hiding this comment.
you put deleteUser(user1Id); before all assertions
There was a problem hiding this comment.
I understood what you suggested, but couldn't figure out why is that better?
There was a problem hiding this comment.
it is better because because deleteUser(user1Id) will happen even if assertion fails, so we perform cleanup any way.
Not critical.
|
@asisayag2 This looks much better now, the only instance in one test that failed in Travis is unrelated: but this is one build instance out of 20 that failed, looks like a timeout. |
Today, we have failing tests that are caused from users that are created during build time and are not cleaned-up for some reason.
For reducing the probability of this to happen, I added a delete user call at the end of every relevant test.
In addition, the TearDown also cleans up users in case some of them were skipped/failed during the test itself.