Correcting Logic for archive/unarchive collection process - #339
Conversation
|
I'd like to see some kind of addition to an existing test that covers this case |
Per our meeting time, I'd like to see a unit test that flexes the execution paths in this case statement. It's OK to adjust the scope. Protected would be OK if your test has the right scope. |
| PublishStatus.Workspace -> { | ||
| dao.status = PublishStatus.Workspace | ||
| } | ||
| else -> {} |
There was a problem hiding this comment.
Can this case ever happen? CollectionDao.status is not nullable, and the dao argument is also not nullable.
There was a problem hiding this comment.
yeah I just supposed that this was like a default in java case statement, I have heard that it is a good practice to leave not skip the default even if nothing needs to happen there.
| @Autowired | ||
| lateinit var collectionController: CollectionController | ||
|
|
||
| var authentication: Authentication = Mockito.mock(Authentication::class.java) |
There was a problem hiding this comment.
In Kotlin, we often use mockk. I can't give you a deep reason why, but there are times/places where Mockito is problematic. I'm not asking for a change here, just please be aware.
There was a problem hiding this comment.
Yeah I know, I just replicated a code that I already had before knowing about mockk, and since Mockito was working for the needed here I just used it here.
| author = "newAuthor") | ||
| val collection = collectionRepository.create(name = "name", user = "user", email = "j.chavez@wgu.edu", description = "description") | ||
| collection!!.status = PublishStatus.Archived | ||
| collection!!.archiveDate = LocalDateTime.now() |
There was a problem hiding this comment.
IDEA reports that collection!! isn't needed on line 106
There was a problem hiding this comment.
Yeah, the assertion is already in line 105, let me remove it.
| description = "newDescription", | ||
| publishStatus = PublishStatus.Unarchived, | ||
| author = "newAuthor") | ||
| val collection = collectionRepository.create(name = "name", user = "user", email = "j.chavez@wgu.edu", description = "description") |
There was a problem hiding this comment.
Can we please not use real identities in source code?
There was a problem hiding this comment.
Correcting this..
| } | ||
|
|
||
| @Test | ||
| fun `applyStatusChange() should apply Published status`() { |
There was a problem hiding this comment.
This test name does not seem to align with the assertion? Let's straighten this out please?
4 participants
Uh oh!
There was an error while loading. Please reload this page.