From 01ae2b047fe451b6c011fc5ef54ec330ef013040 Mon Sep 17 00:00:00 2001 From: ibaker Date: Mon, 11 Jan 2021 12:08:35 +0000 Subject: [PATCH] Remove `expectedResolvedLength` from `DataSourceContractTest` Whether a resource resolves to a known length or not is more than just a property of the resource & data source - for example if `DataSpec.flags` contains `ALLOW_GZIP` then the length might be unresolved. More generally, a `DataSource` could randomly return `C.UNKNOWN_LENGTH` from `open()` 50% of the time and still fulfil the `DataSource` interface. This makes it ~impossible to write a meaningful assertion aroun this. So this change relaxes the assertion slightly to more closely match the definition of the `DataSource` interface. We leave the `resolveToUnknownLength` toggle in `WebServerDispatcher.Resource` because this is still useful for simulating the case of a server that is serving a file it doesn't know the length of. PiperOrigin-RevId: 351124246 --- .../ContentDataSourceContractTest.java | 1 - .../DefaultHttpDataSourceContractTest.java | 1 - .../upstream/UdpDataSourceContractTest.java | 1 - .../testutil/DataSourceContractTest.java | 43 +++---------------- .../testutil/WebServerDispatcher.java | 10 ++--- 5 files changed, 11 insertions(+), 45 deletions(-) diff --git a/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/ContentDataSourceContractTest.java b/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/ContentDataSourceContractTest.java index e2f082d1d9..1fc7f8a78a 100644 --- a/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/ContentDataSourceContractTest.java +++ b/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/ContentDataSourceContractTest.java @@ -48,7 +48,6 @@ public final class ContentDataSourceContractTest extends DataSourceContractTest .setName("simple (pipe=true)") .setUri(TestContentProvider.buildUri(DATA_PATH, /* pipeMode= */ true)) .setExpectedBytes(completeData) - .setResolvesToUnknownLength(true) .build()); } diff --git a/library/core/src/test/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSourceContractTest.java b/library/core/src/test/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSourceContractTest.java index 07221d64df..92fd23e0a7 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSourceContractTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSourceContractTest.java @@ -135,7 +135,6 @@ public class DefaultHttpDataSourceContractTest extends DataSourceContractTest { .setName(name) .setUri(Uri.parse(urlResolver.apply(resource.getPath()).toString())) .setExpectedBytes(resource.getData()) - .setResolvesToUnknownLength(resource.resolvesToUnknownLength()) .setEndOfInputExpected(!resource.resolvesToUnknownLength()) .build(); } diff --git a/library/core/src/test/java/com/google/android/exoplayer2/upstream/UdpDataSourceContractTest.java b/library/core/src/test/java/com/google/android/exoplayer2/upstream/UdpDataSourceContractTest.java index 97d611dbec..c8dd082e67 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/upstream/UdpDataSourceContractTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/upstream/UdpDataSourceContractTest.java @@ -60,7 +60,6 @@ public class UdpDataSourceContractTest extends DataSourceContractTest { .setName("local-udp-unicast-socket") .setUri(Uri.parse("udp://localhost:" + findFreeUdpPort())) .setExpectedBytes(data) - .setResolvesToUnknownLength(true) .setEndOfInputExpected(false) .build()); } diff --git a/testutils/src/main/java/com/google/android/exoplayer2/testutil/DataSourceContractTest.java b/testutils/src/main/java/com/google/android/exoplayer2/testutil/DataSourceContractTest.java index f07baf97ac..d83387a6b2 100644 --- a/testutils/src/main/java/com/google/android/exoplayer2/testutil/DataSourceContractTest.java +++ b/testutils/src/main/java/com/google/android/exoplayer2/testutil/DataSourceContractTest.java @@ -93,7 +93,9 @@ public abstract class DataSourceContractTest { ? Util.readToEnd(dataSource) : Util.readExactly(dataSource, resource.getExpectedBytes().length); - assertThat(length).isEqualTo(resource.getExpectedResolvedLength()); + if (length != C.LENGTH_UNSET) { + assertThat(length).isEqualTo(resource.getExpectedBytes().length); + } assertThat(data).isEqualTo(resource.getExpectedBytes()); } finally { dataSource.close(); @@ -120,8 +122,8 @@ public abstract class DataSourceContractTest { ? Util.readToEnd(dataSource) : Util.readExactly(dataSource, resource.getExpectedBytes().length - 3); - if (resource.getExpectedResolvedLength() != C.LENGTH_UNSET) { - assertThat(length).isEqualTo(resource.getExpectedResolvedLength() - 3); + if (length != C.LENGTH_UNSET) { + assertThat(length).isEqualTo(resource.getExpectedBytes().length - 3); } byte[] expectedData = Arrays.copyOfRange(resource.getExpectedBytes(), 3, resource.getExpectedBytes().length); @@ -219,19 +221,13 @@ public abstract class DataSourceContractTest { @Nullable private final String name; private final Uri uri; private final byte[] expectedBytes; - private final boolean resolvesToUnknownLength; private final boolean endOfInputExpected; private TestResource( - @Nullable String name, - Uri uri, - byte[] expectedBytes, - boolean resolvesToUnknownLength, - boolean endOfInputExpected) { + @Nullable String name, Uri uri, byte[] expectedBytes, boolean endOfInputExpected) { this.name = name; this.uri = uri; this.expectedBytes = expectedBytes; - this.resolvesToUnknownLength = resolvesToUnknownLength; this.endOfInputExpected = endOfInputExpected; } @@ -251,16 +247,6 @@ public abstract class DataSourceContractTest { return expectedBytes; } - /** - * Returns the expected resolved length of this resource. - * - *

This is either {@link #getExpectedBytes() getExpectedBytes().length} or {@link - * C#LENGTH_UNSET}. - */ - public long getExpectedResolvedLength() { - return resolvesToUnknownLength ? C.LENGTH_UNSET : expectedBytes.length; - } - /** * Returns whether {@link DataSource#read} is expected to return {@link C#RESULT_END_OF_INPUT} * after all the resource data are read. @@ -274,7 +260,6 @@ public abstract class DataSourceContractTest { private @MonotonicNonNull String name; private @MonotonicNonNull Uri uri; private byte @MonotonicNonNull [] expectedBytes; - private boolean resolvesToUnknownLength; private boolean endOfInputExpected; /** Construct a new instance. */ @@ -307,16 +292,6 @@ public abstract class DataSourceContractTest { return this; } - /** - * Sets whether {@link DataSource#open(DataSpec)} is expected to return {@link C#LENGTH_UNSET} - * when passed the URI of this resource and a {@link DataSpec} with {@code length == - * C.LENGTH_UNSET}. - */ - public Builder setResolvesToUnknownLength(boolean value) { - this.resolvesToUnknownLength = value; - return this; - } - /** * Sets whether {@link DataSource#read} is expected to return {@link C#RESULT_END_OF_INPUT} * after all the resource data have been read. By default, this is set to {@code true}. @@ -328,11 +303,7 @@ public abstract class DataSourceContractTest { public TestResource build() { return new TestResource( - name, - checkNotNull(uri), - checkNotNull(expectedBytes), - resolvesToUnknownLength, - endOfInputExpected); + name, checkNotNull(uri), checkNotNull(expectedBytes), endOfInputExpected); } } } diff --git a/testutils/src/main/java/com/google/android/exoplayer2/testutil/WebServerDispatcher.java b/testutils/src/main/java/com/google/android/exoplayer2/testutil/WebServerDispatcher.java index aa55de95fc..62dcc55f15 100644 --- a/testutils/src/main/java/com/google/android/exoplayer2/testutil/WebServerDispatcher.java +++ b/testutils/src/main/java/com/google/android/exoplayer2/testutil/WebServerDispatcher.java @@ -91,6 +91,9 @@ public class WebServerDispatcher extends Dispatcher { /** * Sets if the resource should resolve to an unknown length. Defaults to false. * + *

If true, responses to unbound requests won't include a Content-Length header and + * Content-Range headers won't include the total resource length. + * * @return this builder, for convenience. */ public Builder resolvesToUnknownLength(boolean resolvesToUnknownLength) { @@ -133,12 +136,7 @@ public class WebServerDispatcher extends Dispatcher { return supportsRangeRequests; } - /** - * Returns true if the server shouldn't include the resource length in header responses. - * - *

Responses to unbound requests won't include a Content-Length header, and Content-Range - * headers won't include the total resource length. - */ + /** Returns true if the resource should resolve to an unknown length. */ public boolean resolvesToUnknownLength() { return resolvesToUnknownLength; }