Add unit tests for UploadActionUseCase

[malinajirka]

This PR adds unit tests for UploadActionUseCase class.

To test:
CircleCI will take care of running the tests.

Update release notes:

• There are no user facing changes

[peril-wordpress-mobile]

	Warnings
⚠️	PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS

[shiki]

This looks great, @malinajirka! 🎉 It looks like you covered this 100%. I have a small suggestion for one of the test methods for your consideration. Feel free to deal with it however you want though. 😄 This is already perfect for me. Please merge whenever you're ready.

[shiki]

I had expected that we would sill upload local drafts but I guess DO_NOTHING is safer. In any case, I think we should cover permutations that we consider as important. For this test, what do you think of this?

    @Test
    fun `autoUploadAction is DO NOTHING when the post is older than 2 days`() {
        // Arrange
        val uploadActionUseCase = createUploadActionUseCase()

        val twoDaysAgoTimestamp = run {
            val twoDaysInSeconds = 60 * 60 * 24 * 2
            val twoDaysAgo = (Date().time / 1000) - twoDaysInSeconds
            DateTimeUtils.iso8601FromTimestamp(twoDaysAgo)
        }

        val posts = listOf(
                createPostModel(dateLocallyChanged = twoDaysAgoTimestamp),
                createPostModel(dateLocallyChanged = twoDaysAgoTimestamp, isLocalDraft = true, changesConfirmed = true),
                createPostModel(dateLocallyChanged = twoDaysAgoTimestamp, isLocalDraft = false, changesConfirmed = true)
        )

        val siteModel: SiteModel = createSiteModel()

        // Act and Assert
        assertThat(posts).allSatisfy { post ->
            val action = uploadActionUseCase.getAutoUploadAction(post, siteModel)

            assertThat(action).isEqualTo(UploadAction.DO_NOTHING)
        }
    }

[malinajirka]

I had expected that we would sill upload local drafts

My understanding was that one of the main reasons why we want to have this "2 day" guard is that if the upload doesn't work and the user decides to update the post on the web, we don't want to override the remote post after that. If we uploaded it as a draft we might "un-publish" a post. So as you said I think it's safer to simply ignore it.

I think we should cover permutations that we consider as important. For this test, what do you think of this?

Tbh I'm not sure about it. Can you elaborate why you think it's important to test the permutations just for this specific scenario and not for all the others (especially since the 2 day guard is just additional guard which isn't critical at all)? I mean ideally we'd cover 100% of all the possible scenarios, but I don't think it's the goal of our unit tests. I agree that you could change the getAutoUploadAction so the tests would still succeed and the logic would be broken, but you would need to put an effort into it - eg. add conditions like if(isOlderThan2Days && !isLocalDraft) or similar.
Either case I added the new test as it can't hurt us to have more unit tests. I'd still like to hear your opinion;). Thanks

[shiki]

Thank you, @malinajirka.

I don't think it's important to cover all scenarios. Just the important ones. And, for me, it's important to cover the scenario that “local drafts will also not be automatically uploaded if they're 2 days old”. This felt important to me because when reviewing this, as I mentioned, I had expected that local drafts will always be automatically uploaded. But apparently, we don't.

And it's fine that we don't auto-upload, I just requested that it will be added in the test to document that this is how we intended it to work. 🙂

Please let me know if I didn't explain this well. 😅

[malinajirka]

Makes sense;). Thanks for the clarification!

[malinajirka]

Thanks @shiki ;)!

[shiki]

Thank you, @malinajirka! 🚢