Fix NPE when listing files in screenshot folder

[rafakob]

listFiles() may return null which leads to crash:

Exception in thread "main" java.lang.IllegalArgumentException: Parameter specified as non-null is null: method kotlin.collections.ArraysKt___ArraysKt.toList, parameter $receiver
        at kotlin.collections.ArraysKt___ArraysKt.toList(_Arrays.kt)
        at com.gojuno.composer.TestRunKt$pullTestFiles$3.call(TestRun.kt:177)
        at com.gojuno.composer.TestRunKt$pullTestFiles$3.call(TestRun.kt)

[koral--]

listFiles() can return null only if it is not a directory. Calling exists() beforehand is partially redundant IMHO.
There is also a race condition possible when directory is removed in the time between two listFiles() calls.

What about replacing when block with it.listFiles()?.toList() ?: emptyList() ?

[artem-zinnatullin]

What about when (it.exists() && it.isDirectory())?

Race condition is still possible with it.listFiles()?.toList() ?: emptyList() since we're going to get problems later during adb pull if list of files is invalid

[koral--]

isDirectory() returns false if there is no directory under given path including the case when there is nothing there.

we're going to get problems later during adb pull if list of files is invalid

It seems that screenshots have been already pulled when we are listing those files:
https://github.com/rafakob/composer/blob/f70e773c73adc6e7f98ffaf85f0000d5f8675ee4/composer/src/main/kotlin/com/gojuno/composer/TestRun.kt#L164

[artem-zinnatullin]

ah right, my bad

returns false if there is no directory under given path including the case when there is nothing there.

Isn't that what we want?

[koral--]

Yes, but listFiles() != null also checks for existence.

[artem-zinnatullin]

Well for sake of removing redundancy from if statement I'm 👍 with using listFiles() != null

[plastiv]

How to reproduce this NPE?

it.listFiles() can return null only when it is file instead of folder. Who is creating it as a file?

Composer code doesn't seem to be touching it, creating parent testClassName folder only. So it looks like file was downloaded by adb pull. Which could happen only when not complying to spoon screenshot contract

storage/sdcard/app_spoon-screenshots/package.testClassName/testMethodName/screenshotName.png`

In this case, I think proposed solution to silently ignoring NPE is incorrect. Because storage/sdcard/app_spoon-screenshots/package.testClassName/testMethodName screenshot is not getting merged into html report. Instead, I would prefer to have an assertion message to avoid debugging why.