From 635544efe9e8a16ddadb9814247e1f61ce923544 Mon Sep 17 00:00:00 2001
From: olly <olly@google.com>
Date: Thu, 25 Feb 2021 14:02:17 +0000
Subject: [PATCH] Add DataSourceContractTest for reading from the end with a
 set length

Given we're proposing to make reading from the end a non-error case,
it's important to check that we return the right thing from open(),
and that we read the right thing (i.e., nothing) once opened.

For now, this test allows quite a bit of permissiveness, in line
with other related tests. This will be tightened up in due course.

PiperOrigin-RevId: 359504075
---
 .../upstream/UdpDataSourceContractTest.java   |  7 ++-
 .../testutil/DataSourceContractTest.java      | 51 ++++++++++++++++---
 2 files changed, 50 insertions(+), 8 deletions(-)

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 6a3441927c..af9159088f 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
@@ -91,7 +91,12 @@ public class UdpDataSourceContractTest extends DataSourceContractTest {
   @Test
   @Ignore("UdpDataSource doesn't support DataSpec's position or length [internal: b/175856954]")
   @Override
-  public void dataSpecWithPositionEqualToLength_throwsPositionOutOfRangeException() {}
+  public void dataSpecWithPositionAtEnd_throwsPositionOutOfRangeException() {}
+
+  @Test
+  @Ignore("UdpDataSource doesn't support DataSpec's position or length [internal: b/175856954]")
+  @Override
+  public void dataSpecWithPositionAtEndAndLength_throwsPositionOutOfRangeException() {}
 
   @Test
   @Ignore("UdpDataSource doesn't support DataSpec's position or length [internal: b/175856954]")
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 7da680a6d2..120f758a7e 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
@@ -214,8 +214,7 @@ public abstract class DataSourceContractTest {
   }
 
   @Test
-  public void dataSpecWithPositionEqualToLength_throwsPositionOutOfRangeException()
-      throws Exception {
+  public void dataSpecWithPositionAtEnd_throwsPositionOutOfRangeException() throws Exception {
     ImmutableList<TestResource> resources = getTestResources();
     Assertions.checkArgument(!resources.isEmpty(), "Must provide at least one test resource.");
 
@@ -227,9 +226,51 @@ public abstract class DataSourceContractTest {
       DataSpec dataSpec =
           new DataSpec.Builder().setUri(resource.getUri()).setPosition(resourceLength).build();
       try {
-        dataSource.open(dataSpec);
+        long length = dataSource.open(dataSpec);
         // TODO: For any cases excluded from the requirement that a position-out-of-range exception
         // is thrown, decide what the allowed behavior should be for the first read, and assert it.
+
+        if (length != C.LENGTH_UNSET) {
+          assertThat(length).isEqualTo(0);
+        }
+      } catch (IOException e) {
+        // TODO: Decide whether to assert that a position-out-of-range exception must or must not be
+        // thrown (with exclusions if necessary), rather than just asserting it must be a
+        // position-out-of-range exception *if* one is thrown at all.
+        assertThat(DataSourceException.isCausedByPositionOutOfRange(e)).isTrue();
+      } finally {
+        dataSource.close();
+      }
+      additionalFailureInfo.setInfo(null);
+    }
+  }
+
+  @Test
+  public void dataSpecWithPositionAtEndAndLength_throwsPositionOutOfRangeException()
+      throws Exception {
+    ImmutableList<TestResource> resources = getTestResources();
+    Assertions.checkArgument(!resources.isEmpty(), "Must provide at least one test resource.");
+
+    for (int i = 0; i < resources.size(); i++) {
+      additionalFailureInfo.setInfo(getFailureLabel(resources, i));
+      TestResource resource = resources.get(i);
+      int resourceLength = resource.getExpectedBytes().length;
+      DataSource dataSource = createDataSource();
+      DataSpec dataSpec =
+          new DataSpec.Builder()
+              .setUri(resource.getUri())
+              .setPosition(resourceLength)
+              .setLength(1)
+              .build();
+      try {
+        long length = dataSource.open(dataSpec);
+        // TODO: For any cases excluded from the requirement that a position-out-of-range exception
+        // is thrown, decide what the allowed behavior should be for the first read, and assert it.
+
+        // The DataSource.open() contract requires the returned length to equal the length in the
+        // DataSpec if set. This is true even though the DataSource implementation may know that
+        // fewer bytes will be read in this case.
+        assertThat(length).isEqualTo(1);
       } catch (IOException e) {
         // TODO: Decide whether to assert that a position-out-of-range exception must or must not be
         // thrown (with exclusions if necessary), rather than just asserting it must be a
@@ -412,10 +453,6 @@ public abstract class DataSourceContractTest {
     }
   }
 
-  // TODO: This works around [Internal ref: b/174231044]. Remove when possible.
-  @Test
-  public void emptyTest() {}
-
   /** Build a label to make it clear which resource caused a given test failure. */
   private static String getFailureLabel(List<TestResource> resources, int i) {
     if (resources.size() == 1) {