alex-rufous commented on issue #42: Qpid-8374: [Broker-J][ACL] Allow case 
insensitive mapping of group members to groups in existing GroupProvider
URL: https://github.com/apache/qpid-broker-j/pull/42#issuecomment-560462637
 
 
   Hi Stanislav,
   Here some review comments on  changes in 
FileGroupDatabaseCaseInsensitiveTest/FileGroupDatabaseTest:
   
   - Setting constant variables in @BeforeClass is unnecessary complication. 
The user and group test data seems are constants and should remain declared as 
constant. The reason why I asked to introduce @BeforeClass/@AfterClass methods 
is just to remove test files immediately after the test
   - I noticed that call to `UTIL.writeAndSetGroupFile(...)` is followed by 
`FILE_GROUP_DATABASE.setGroupFile(GROUP_FILE)` in some test cases.  That seems 
unnecessary as `FileGroupDatabase#setGroupFile` is called inside 
`UTIL.writeAndSetGroupFile()`
   - There is no need to catch IOException in @BeforeClass and re-throw it as 
RuntimeException. Please change the method to throw Exception from  
@BeforeClass 
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to