MOB ID-440 Refactor/engooden CreateUserSpec and InviteUserspec. #998
Conversation
…o mob-ID-428-invite-user-spec
# Conflicts: # src/test/scala/org/broadinstitute/dsde/workbench/sam/service/UserServiceSpec.scala
|
Kudos, SonarCloud Quality Gate passed!
|
| "allUsersGroup" should beEnabledIn(status) | ||
| "adminEnabled" should beEnabledIn(status) | ||
| "tosAccepted" shouldNot beEnabledIn(status) | ||
| val userWithBoth = (genWorkbenchUserBoth.sample.get, "user with google and azure ids") |
There was a problem hiding this comment.
I like the tuples, my only concern is that it makes it a little harder to read it in the code - although the test output is still clear. Maybe we could add a comment or a readme somewhere suggesting that as an easy way to debug/check expected behavior? Maybe everyone would think to do that, but I'm not used to the test results printing out in such a helpful manner so I'm not sure if it would occur to me if I were a dev looking at this for the first time.
| describe("that does not already exist") { | ||
| newUsers.foreach { userTuple => | ||
| val (newUser, description) = userTuple | ||
| describe(s"UserService.createUser with a $description") { |
There was a problem hiding this comment.
Nit: can we reword this message a tiny bit? I find the phrasing of "when a new user… that does not already exist… Userservice.createUser with a user with google and azure ids" more confusing than it was before
There was a problem hiding this comment.
I think it needs one additional verb or something
Ticket: https://broadworkbench.atlassian.net/browse/ID-440
branches off of #994