Revert "Merge pull request #24752 from vespa-engine/jonmv/fix-test-packages"

This reverts commit 3aca43e7f71c54bb2ab61bc495f85e535d4cd3fd, reversing
changes made to 1c48ffbdc847a328dbc2b45019628135939c8012.

4 files changed, 16 insertions, 52 deletions

diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java b/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java
index f69bd5fa475..c9ec9780fad 100644
--- a/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java
+++ b/config-model-api/src/main/java/com/yahoo/config/application/api/DeploymentSpec.java
@@ -194,7 +194,7 @@ public class DeploymentSpec {
     public DeploymentInstanceSpec requireInstance(InstanceName name) {
         Optional<DeploymentInstanceSpec> instance = instance(name);
         if (instance.isEmpty())
-            throw new IllegalArgumentException("No instance '" + name + "' in deployment.xml. Instances: " +
+            throw new IllegalArgumentException("No instance '" + name + "' in deployment.xml'. Instances: " +
                                                instances().stream().map(spec -> spec.name().toString()).collect(Collectors.joining(",")));
         return instance.get();
     }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ZipEntries.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ZipEntries.java
index 63915c5050f..6bbcd551924 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ZipEntries.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ZipEntries.java
@@ -15,7 +15,6 @@ import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
-import java.util.NoSuchElementException;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.function.Predicate;
@@ -108,7 +107,7 @@ public class ZipEntries {
         }
 
         public String name() { return name; }
-        public byte[] contentOrThrow() { return content.orElseThrow(() -> new NoSuchElementException("'" + name + "' has no content")); }
+        public byte[] contentOrThrow() { return content.orElseThrow(); }
         public Optional<byte[]> content() { return content; }
         public long size() { return size; }
     }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageTest.java
index 71ce291df36..609b7abf5f0 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageTest.java
@@ -160,11 +160,11 @@ public class ApplicationPackageTest {
         assertEquals(originalPackage.bundleHash(), similarDeploymentXml.bundleHash());
     }
 
-    static Map<String, String> unzip(byte[] zip) {
-        return ZipEntries.from(zip, __ -> true, 1 << 24, true)
+    private static Map<String, String> unzip(byte[] zip) {
+        return ZipEntries.from(zip, __ -> true, 1 << 10, true)
                          .asList().stream()
                          .collect(Collectors.toMap(ZipEntries.ZipEntryWithContent::name,
-                                                   entry -> new String(entry.content().orElse(new byte[0]), UTF_8)));
+                                          entry -> new String(entry.contentOrThrow(), UTF_8)));
     }
 
     private ApplicationPackage getApplicationZip(String path) throws IOException {
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/TestPackageTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/TestPackageTest.java
index 6da8db1c259..bff0ccc8ae1 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/TestPackageTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/TestPackageTest.java
@@ -1,24 +1,18 @@
 package com.yahoo.vespa.hosted.controller.application.pkg;
 
 import com.yahoo.config.application.api.DeploymentSpec;
-import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.NodeResources;
 import com.yahoo.config.provision.zone.ZoneId;
-import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType;
-import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId;
 import com.yahoo.vespa.hosted.controller.application.pkg.TestPackage.TestSummary;
 import com.yahoo.vespa.hosted.controller.config.ControllerConfig;
-import com.yahoo.vespa.hosted.controller.config.ControllerConfig.Steprunner.Testerapp;
 import org.junit.jupiter.api.Test;
 
-import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Paths;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 import java.util.jar.JarOutputStream;
 import java.util.zip.ZipEntry;
 
@@ -26,11 +20,9 @@ import static com.yahoo.vespa.hosted.controller.api.integration.deployment.Teste
 import static com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterCloud.Suite.staging;
 import static com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterCloud.Suite.staging_setup;
 import static com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterCloud.Suite.system;
-import static com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackageTest.unzip;
 import static com.yahoo.vespa.hosted.controller.application.pkg.TestPackage.validateTests;
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertTrue;
 
 /**
  * @author jonmv
@@ -85,15 +77,15 @@ public class TestPackageTest {
     @Test
     void testBundleValidation() throws IOException {
         byte[] testZip = ApplicationPackage.filesZip(Map.of("components/foo-tests.jar", testsJar("SystemTest", "StagingSetup", "ProductionTest"),
-                                                            "artifacts/key", new byte[0]));
+                "artifacts/key", new byte[0]));
         TestSummary summary = validateTests(List.of(system), testZip);
 
         assertEquals(List.of(system, staging_setup, production), summary.suites());
         assertEquals(List.of("test package contains 'artifacts/key'; this conflicts with credentials used to run tests in Vespa Cloud",
-                             "test package has staging setup, so it should also include staging tests",
-                             "test package has production tests, but no production tests are declared in deployment.xml",
-                             "see https://docs.vespa.ai/en/testing.html for details on how to write system tests for Vespa"),
-                     summary.problems());
+                        "test package has staging setup, so it should also include staging tests",
+                        "test package has production tests, but no production tests are declared in deployment.xml",
+                        "see https://docs.vespa.ai/en/testing.html for details on how to write system tests for Vespa"),
+                summary.problems());
     }
 
     @Test
@@ -103,47 +95,20 @@ public class TestPackageTest {
 
         assertEquals(List.of(staging, production), summary.suites());
         assertEquals(List.of("test package has staging tests, so it should also include staging setup",
-                             "see https://docs.vespa.ai/en/testing.html for details on how to write system tests for Vespa"),
-                     summary.problems());
+                        "see https://docs.vespa.ai/en/testing.html for details on how to write system tests for Vespa"),
+                summary.problems());
     }
 
     @Test
     void testBasicTestsValidation() {
         byte[] testZip = ApplicationPackage.filesZip(Map.of("tests/staging-test/foo.json", new byte[0],
-                                                            "tests/staging-setup/foo.json", new byte[0]));
+                "tests/staging-setup/foo.json", new byte[0]));
         TestSummary summary = validateTests(List.of(system, production), testZip);
         assertEquals(List.of(staging_setup, staging), summary.suites());
         assertEquals(List.of("test package has no system tests, but <test /> is declared in deployment.xml",
-                             "test package has no production tests, but production tests are declared in deployment.xml",
-                             "see https://docs.vespa.ai/en/testing.html for details on how to write system tests for Vespa"),
-                     summary.problems());
-    }
-
-    @Test
-    void testTestPacakgeAssembly() throws IOException {
-        byte[] bundleZip = ApplicationPackage.filesZip(Map.of("components/foo-tests.jar", testsJar("SystemTest", "ProductionTest"),
-                                                              "artifacts/key", new byte[0]));
-        TestPackage bundleTests = new TestPackage(() -> new ByteArrayInputStream(bundleZip),
-                                                  false,
-                                                  new RunId(ApplicationId.defaultId(), JobType.dev("abc"), 123),
-                                                  new Testerapp.Builder().tenantCdBundle("foo").runtimeProviderClass("bar").build(),
-                                                  DeploymentSpec.fromXml("""
-                                                                         <deployment>
-                                                                           <test />
-                                                                         </deployment>
-                                                                         """),
-                                                  null,
-                                                  null);
-
-        Map<String, String> bundlePackage = unzip(bundleTests.asApplicationPackage().zipStream().readAllBytes());
-        bundlePackage.keySet().removeIf(name -> name.startsWith("tests/.ignore") || name.startsWith("artifacts/.ignore"));
-        assertEquals(Set.of("deployment.xml",
-                            "services.xml",
-                            "components/foo-tests.jar",
-                            "artifacts/key"),
-                     bundlePackage.keySet());
-        assertEquals(Map.of(),
-                     unzip(bundleTests.asApplicationPackage().truncatedPackage().zippedContent()));
+                        "test package has no production tests, but production tests are declared in deployment.xml",
+                        "see https://docs.vespa.ai/en/testing.html for details on how to write system tests for Vespa"),
+                summary.problems());
     }
 
     @Test