Wordpress-android: [Spike] Automated tests for upload flow

Created on 8 May 2019  路  7Comments  路  Source: wordpress-mobile/WordPress-Android

As I was working on #9804, I found it difficult to understand and follow the upload flow of UploadService, MediaUploadHandler, PostUploadHandler, and their FluxC counterparts. I was also worried about touching them because I could not find any automated tests. As an example, I removed this line in FluxC. It looked critical but the tests still passed.

I propose that before we do any changes to these classes, particularly as a result of #9803, we should:

  • Make them testable. We'll probably have to split them up into multiple UseCase classes.
  • Write tests.
  • Document how uploading is supposed to work.

In the future, we could possibly convert these to use coroutines too.

2 Offline Support Testing [Type] Enhancement

Most helpful comment

Note: the 2 days estimation is a "spike task", ie. we'll spend 2 days to explore and check if we can make these classes testable.

All 7 comments

I had to make some changes to the UploadService in the last couple of days and I think we should consider giving this ticket a higher priority. Thinking of all the scenarios and making sure nothing breaks with the changes is difficult or error prone at least. Wdyt @maxme?

@malinajirka do you see any ticket in our current backlog that would require changes in the UploadService? If so, yes let's bump this one and add it as a dependency to these tickets.

@maxme There are two open PRs which are touching the UploadService - #10119 and #10079 and I was also wondering if introduction of "remote-auto-save" won't need to make some changes as well. Wdyt?

Ok, so let's bump this to high priority. We can do a one day spike to make UploadService testable:

  • if it needs more than that, we should just write a comment/post with recommendations.
  • if we can make it testable, we can continue on this and write some tests.

Note: the 2 days estimation is a "spike task", ie. we'll spend 2 days to explore and check if we can make these classes testable.

I'm checking in on high priority issues and am removing the priority label in this case due to inactivity. If this is incorrect and the issue should still be high priority, please re-add the label and move it to an active project board. 馃檹 cc @maxme @yaelirub

@designsimply sounds good, thank you!

Was this page helpful?
0 / 5 - 0 ratings