Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ test_konsist = "com.lemonappdev:konsist:0.17.3"
test_turbine = "app.cash.turbine:turbine:1.2.1"
test_truth = "com.google.truth:truth:1.4.4"
test_parameter_injector = "com.google.testparameterinjector:test-parameter-injector:1.18"
test_robolectric = "org.robolectric:robolectric:4.15.1"
test_robolectric = "org.robolectric:robolectric:4.16"
test_appyx_junit = { module = "com.bumble.appyx:testing-junit4", version.ref = "appyx" }
test_composable_preview_scanner = "io.github.sergio-sastre.ComposablePreviewScanner:android:0.7.0"
test_detekt_api = { module = "io.gitlab.arturbosch.detekt:detekt-api", version.ref = "detekt" }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ class AndroidFileSizeFormatter @Inject constructor(
} else {
// First convert the size
when {
fileSize < 1024 -> fileSize
fileSize <= 1 -> fileSize
fileSize < 1024 -> fileSize * 1000 / 1024
fileSize < 1024 * 1024 -> fileSize * 1000 / 1024
fileSize < 1024 * 1024 * 1024 -> fileSize * 1000 / 1024 * 1000 / 1024
else -> fileSize * 1000 / 1024 * 1000 / 1024 * 1000 / 1024
Expand All @@ -40,6 +41,6 @@ class AndroidFileSizeFormatter @Inject constructor(
Formatter.formatShortFileSize(context, normalizedSize)
} else {
Formatter.formatFileSize(context, normalizedSize)
}
}.replace("kB", "KB")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this replacement? It seems that kB is more correct than KB, since k is the official shortcut for kilo in the international system of units.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then I understood the comments on that class and the existing tests backwards, I thought we wanted to keep the KB and 1024 base instead:

// Since Android O, the system considers that 1kB = 1000 bytes instead of 1024 bytes. We want to avoid that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the end did we decide on whether to use 1024 or 1000? I think it was 1000?

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,45 +14,52 @@ import org.junit.Test
import org.junit.runner.RunWith
import org.robolectric.RobolectricTestRunner
import org.robolectric.RuntimeEnvironment
import org.robolectric.annotation.Config

@RunWith(RobolectricTestRunner::class)
class AndroidFileSizeFormatterTest {
@Config(sdk = [Build.VERSION_CODES.N])
@Test
fun `test api 24 long format`() {
val sut = createAndroidFileSizeFormatter(sdkLevel = Build.VERSION_CODES.N)
assertThat(sut.format(1, useShortFormat = false)).isEqualTo("1.00B")
assertThat(sut.format(1000, useShortFormat = false)).isEqualTo("0.98KB")
assertThat(sut.format(1024, useShortFormat = false)).isEqualTo("1.00KB")
assertThat(sut.format(1024 * 1024, useShortFormat = false)).isEqualTo("1.00MB")
assertThat(sut.format(1024 * 1024 * 1024, useShortFormat = false)).isEqualTo("1.00GB")
assertThat(sut.format(1, useShortFormat = false)).isEqualTo("1 B")
assertThat(sut.format(1000, useShortFormat = false)).isEqualTo("0.98 KB")
assertThat(sut.format(1024, useShortFormat = false)).isEqualTo("1.00 KB")
assertThat(sut.format(1024 * 1024, useShortFormat = false)).isEqualTo("1.00 MB")
assertThat(sut.format(1024 * 1024 * 1024, useShortFormat = false)).isEqualTo("1.00 GB")
}

@Config(sdk = [Build.VERSION_CODES.O])
@Test
fun `test api 26 long format`() {
val sut = createAndroidFileSizeFormatter(sdkLevel = Build.VERSION_CODES.O)
assertThat(sut.format(1, useShortFormat = false)).isEqualTo("1.00B")
assertThat(sut.format(1000, useShortFormat = false)).isEqualTo("0.98KB")
assertThat(sut.format(1024 * 1024, useShortFormat = false)).isEqualTo("0.95MB")
assertThat(sut.format(1024 * 1024 * 1024, useShortFormat = false)).isEqualTo("0.93GB")
assertThat(sut.format(1, useShortFormat = false)).isEqualTo("1 B")
assertThat(sut.format(1000, useShortFormat = false)).isEqualTo("0.98 KB")
assertThat(sut.format(1024, useShortFormat = false)).isEqualTo("1.00 KB")
assertThat(sut.format(1024 * 1024, useShortFormat = false)).isEqualTo("1.00 MB")
assertThat(sut.format(1024 * 1024 * 1024, useShortFormat = false)).isEqualTo("1.00 GB")
}

@Config(sdk = [Build.VERSION_CODES.N])
@Test
fun `test api 24 short format`() {
val sut = createAndroidFileSizeFormatter(sdkLevel = Build.VERSION_CODES.N)
assertThat(sut.format(1, useShortFormat = true)).isEqualTo("1.0B")
assertThat(sut.format(1000, useShortFormat = true)).isEqualTo("0.98KB")
assertThat(sut.format(1024, useShortFormat = true)).isEqualTo("1.0KB")
assertThat(sut.format(1024 * 1024, useShortFormat = true)).isEqualTo("1.0MB")
assertThat(sut.format(1024 * 1024 * 1024, useShortFormat = true)).isEqualTo("1.0GB")
assertThat(sut.format(1, useShortFormat = true)).isEqualTo("1 B")
assertThat(sut.format(1000, useShortFormat = true)).isEqualTo("0.98 KB")
assertThat(sut.format(1024, useShortFormat = true)).isEqualTo("1.0 KB")
assertThat(sut.format(1024 * 1024, useShortFormat = true)).isEqualTo("1.0 MB")
assertThat(sut.format(1024 * 1024 * 1024, useShortFormat = true)).isEqualTo("1.0 GB")
}

@Config(sdk = [Build.VERSION_CODES.O])
@Test
fun `test api 26 short format`() {
val sut = createAndroidFileSizeFormatter(sdkLevel = Build.VERSION_CODES.O)
assertThat(sut.format(1, useShortFormat = true)).isEqualTo("1.0B")
assertThat(sut.format(1000, useShortFormat = true)).isEqualTo("0.98KB")
assertThat(sut.format(1024 * 1024, useShortFormat = true)).isEqualTo("0.95MB")
assertThat(sut.format(1024 * 1024 * 1024, useShortFormat = true)).isEqualTo("0.93GB")
assertThat(sut.format(1, useShortFormat = true)).isEqualTo("1 B")
assertThat(sut.format(1000, useShortFormat = true)).isEqualTo("0.98 KB")
assertThat(sut.format(1024, useShortFormat = true)).isEqualTo("1.0 KB")
assertThat(sut.format(1024 * 1024, useShortFormat = true)).isEqualTo("1.0 MB")
assertThat(sut.format(1024 * 1024 * 1024, useShortFormat = true)).isEqualTo("1.0 GB")
}

private fun createAndroidFileSizeFormatter(sdkLevel: Int) = AndroidFileSizeFormatter(
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change from 100 B to 97 B does not seem correct. And we may choose a different test data than 100 for the maxUploadSize, 100 B is not really realistic.

Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previous value with kB is more correct than the new one with KB.

Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading