From 93b96e176b33aceb5c830be0da6cf1ecbdf93fe7 Mon Sep 17 00:00:00 2001 From: Bjørn Christian Seime Date: Thu, 28 Jul 2022 13:00:58 +0200 Subject: Convert controller-api to junit5 --- controller-api/pom.xml | 16 +- .../controller/api/identifiers/IdentifierTest.java | 167 +++++++++++++-------- .../api/integration/deployment/JobTypeTest.java | 10 +- .../api/integration/dns/AliasTargetTest.java | 8 +- .../api/integration/maven/MetadataTest.java | 6 +- .../controller/api/integration/user/RolesTest.java | 51 ++++--- .../hosted/controller/api/role/PathGroupTest.java | 22 +-- .../vespa/hosted/controller/api/role/RoleTest.java | 48 +++--- .../systemflags/v1/SystemFlagsDataArchiveTest.java | 96 ++++++------ 9 files changed, 238 insertions(+), 186 deletions(-) (limited to 'controller-api') diff --git a/controller-api/pom.xml b/controller-api/pom.xml index f7057c93561..c7ac5d3518a 100644 --- a/controller-api/pom.xml +++ b/controller-api/pom.xml @@ -84,18 +84,22 @@ - - junit - junit - test - - com.yahoo.vespa configdefinitions ${project.version} test + + org.junit.jupiter + junit-jupiter-api + test + + + org.junit.jupiter + junit-jupiter-engine + test + org.mockito diff --git a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/identifiers/IdentifierTest.java b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/identifiers/IdentifierTest.java index 0dcb800ca04..902d260c86d 100644 --- a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/identifiers/IdentifierTest.java +++ b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/identifiers/IdentifierTest.java @@ -2,129 +2,168 @@ package com.yahoo.vespa.hosted.controller.api.identifiers; import com.yahoo.config.provision.zone.ZoneId; -import org.junit.Test; +import org.junit.jupiter.api.Test; -import static org.junit.Assert.assertEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; /** * @author smorgrav */ public class IdentifierTest { - @Test(expected = IllegalArgumentException.class) - public void existing_tenant_id_not_empty() { - new TenantId(""); + @Test + void existing_tenant_id_not_empty() { + assertThrows(IllegalArgumentException.class, () -> { + new TenantId(""); + }); } - @Test(expected = IllegalArgumentException.class) - public void existing_tenant_id_must_check_pattern() { - new TenantId("`"); + @Test + void existing_tenant_id_must_check_pattern() { + assertThrows(IllegalArgumentException.class, () -> { + new TenantId("`"); + }); } - @Test(expected = IllegalArgumentException.class) - public void default_not_allowed_for_tenants() { - new TenantId("default"); + @Test + void default_not_allowed_for_tenants() { + assertThrows(IllegalArgumentException.class, () -> { + new TenantId("default"); + }); } @Test - public void existing_tenant_id_must_accept_valid_id() { + void existing_tenant_id_must_accept_valid_id() { new TenantId("msbe"); } - @Test(expected = IllegalArgumentException.class) - public void existing_tenant_id_cannot_be_uppercase() { - new TenantId("MixedCaseTenant"); + @Test + void existing_tenant_id_cannot_be_uppercase() { + assertThrows(IllegalArgumentException.class, () -> { + new TenantId("MixedCaseTenant"); + }); } - @Test(expected = IllegalArgumentException.class) - public void existing_tenant_id_cannot_contain_dots() { - new TenantId("tenant.with.dots"); + @Test + void existing_tenant_id_cannot_contain_dots() { + assertThrows(IllegalArgumentException.class, () -> { + new TenantId("tenant.with.dots"); + }); } - @Test(expected = IllegalArgumentException.class) - public void new_tenant_id_cannot_contain_underscore() { - TenantId.validate("underscore_tenant"); + @Test + void new_tenant_id_cannot_contain_underscore() { + assertThrows(IllegalArgumentException.class, () -> { + TenantId.validate("underscore_tenant"); + }); } - @Test(expected = IllegalArgumentException.class) - public void new_tenant_id_cannot_contain_dot() { - TenantId.validate("tenant.with.dots"); + @Test + void new_tenant_id_cannot_contain_dot() { + assertThrows(IllegalArgumentException.class, () -> { + TenantId.validate("tenant.with.dots"); + }); } - @Test(expected = IllegalArgumentException.class) - public void new_tenant_id_cannot_contain_uppercase() { - TenantId.validate("UppercaseTenant"); + @Test + void new_tenant_id_cannot_contain_uppercase() { + assertThrows(IllegalArgumentException.class, () -> { + TenantId.validate("UppercaseTenant"); + }); } - @Test(expected = IllegalArgumentException.class) - public void new_tenant_id_cannot_start_with_dash() { - TenantId.validate("-tenant"); + @Test + void new_tenant_id_cannot_start_with_dash() { + assertThrows(IllegalArgumentException.class, () -> { + TenantId.validate("-tenant"); + }); } - @Test(expected = IllegalArgumentException.class) - public void new_tenant_id_cannot_end_with_dash() { - TenantId.validate("tenant-"); + @Test + void new_tenant_id_cannot_end_with_dash() { + assertThrows(IllegalArgumentException.class, () -> { + TenantId.validate("tenant-"); + }); } - @Test(expected = IllegalArgumentException.class) - public void existing_application_id_cannot_be_uppercase() { - new ApplicationId("MixedCaseApplication"); + @Test + void existing_application_id_cannot_be_uppercase() { + assertThrows(IllegalArgumentException.class, () -> { + new ApplicationId("MixedCaseApplication"); + }); } - @Test(expected = IllegalArgumentException.class) - public void existing_application_id_cannot_contain_dots() { - new ApplicationId("application.with.dots"); + @Test + void existing_application_id_cannot_contain_dots() { + assertThrows(IllegalArgumentException.class, () -> { + new ApplicationId("application.with.dots"); + }); } - @Test(expected = IllegalArgumentException.class) - public void new_application_id_cannot_contain_underscore() { - ApplicationId.validate("underscore_application"); + @Test + void new_application_id_cannot_contain_underscore() { + assertThrows(IllegalArgumentException.class, () -> { + ApplicationId.validate("underscore_application"); + }); } - @Test(expected = IllegalArgumentException.class) - public void new_application_id_cannot_contain_dot() { - ApplicationId.validate("application.with.dots"); + @Test + void new_application_id_cannot_contain_dot() { + assertThrows(IllegalArgumentException.class, () -> { + ApplicationId.validate("application.with.dots"); + }); } - @Test(expected = IllegalArgumentException.class) - public void new_application_id_cannot_contain_uppercase() { - ApplicationId.validate("UppercaseApplication"); + @Test + void new_application_id_cannot_contain_uppercase() { + assertThrows(IllegalArgumentException.class, () -> { + ApplicationId.validate("UppercaseApplication"); + }); } - @Test(expected = IllegalArgumentException.class) - public void new_application_id_cannot_start_with_dash() { - ApplicationId.validate("-application"); + @Test + void new_application_id_cannot_start_with_dash() { + assertThrows(IllegalArgumentException.class, () -> { + ApplicationId.validate("-application"); + }); } - @Test(expected = IllegalArgumentException.class) - public void new_application_id_cannot_end_with_dash() { - ApplicationId.validate("application-"); + @Test + void new_application_id_cannot_end_with_dash() { + assertThrows(IllegalArgumentException.class, () -> { + ApplicationId.validate("application-"); + }); } - @Test(expected = IllegalArgumentException.class) - public void instance_id_cannot_be_uppercase() { - new InstanceId("MixedCaseInstance"); + @Test + void instance_id_cannot_be_uppercase() { + assertThrows(IllegalArgumentException.class, () -> { + new InstanceId("MixedCaseInstance"); + }); } @Test - public void dns_names_has_no_underscore() { + void dns_names_has_no_underscore() { assertEquals("a-b-c", new ApplicationId("a_b_c").toDns()); } - @Test(expected = IllegalArgumentException.class) - public void identifiers_cannot_be_named_api() { - new ApplicationId("api"); + @Test + void identifiers_cannot_be_named_api() { + assertThrows(IllegalArgumentException.class, () -> { + new ApplicationId("api"); + }); } @Test - public void application_instance_id_dotted_string_is_subindentifers_concatinated_with_dots() { + void application_instance_id_dotted_string_is_subindentifers_concatinated_with_dots() { DeploymentId id = new DeploymentId(com.yahoo.config.provision.ApplicationId.from("tenant", "application", "instance"), - ZoneId.from("prod", "region")); + ZoneId.from("prod", "region")); assertEquals("tenant.application.prod.region.instance", id.dottedString()); } @Test - public void revision_id_can_contain_application_version_number() { + void revision_id_can_contain_application_version_number() { new RevisionId("1.0.1078-24825d1f6"); } } diff --git a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/JobTypeTest.java b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/JobTypeTest.java index bbe0c2bd458..1693f752b89 100644 --- a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/JobTypeTest.java +++ b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/JobTypeTest.java @@ -2,11 +2,11 @@ package com.yahoo.vespa.hosted.controller.api.integration.deployment; import com.yahoo.config.provision.zone.ZoneId; -import org.junit.Test; +import org.junit.jupiter.api.Test; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; /** * @author jonmv @@ -14,7 +14,7 @@ import static org.junit.Assert.assertTrue; public class JobTypeTest { @Test - public void test() { + void test() { assertEquals(JobType.test("us-east-3"), JobType.ofSerialized("prod.us-east-3.test")); assertEquals(JobType.dev("aws-us-east-1c"), JobType.ofSerialized("dev.aws-us-east-1c")); diff --git a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/dns/AliasTargetTest.java b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/dns/AliasTargetTest.java index 35d2ad1ffca..9cbba8107ef 100644 --- a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/dns/AliasTargetTest.java +++ b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/dns/AliasTargetTest.java @@ -3,12 +3,12 @@ package com.yahoo.vespa.hosted.controller.api.integration.dns; import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.zone.ZoneId; -import org.junit.Test; +import org.junit.jupiter.api.Test; import java.util.List; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.fail; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.fail; /** * @author mpolden @@ -16,7 +16,7 @@ import static org.junit.Assert.fail; public class AliasTargetTest { @Test - public void packing() { + void packing() { List tests = List.of( new LatencyAliasTarget(HostName.of("foo.example.com"), "dns-zone-1", ZoneId.from("prod.us-north-1")), new WeightedAliasTarget(HostName.of("bar.example.com"), "dns-zone-2", ZoneId.from("prod.us-north-2"), 50) diff --git a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/maven/MetadataTest.java b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/maven/MetadataTest.java index 55f33d7e3e1..4cec90eb48d 100644 --- a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/maven/MetadataTest.java +++ b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/maven/MetadataTest.java @@ -2,14 +2,14 @@ package com.yahoo.vespa.hosted.controller.api.integration.maven; import com.yahoo.component.Version; -import org.junit.Test; +import org.junit.jupiter.api.Test; -import static org.junit.Assert.assertEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; public class MetadataTest { @Test - public void testParsing() { + void testParsing() { Metadata metadata = Metadata.fromXml(metadataXml); assertEquals("com.yahoo.vespa", metadata.id().groupId()); assertEquals("tenant-base", metadata.id().artifactId()); diff --git a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/user/RolesTest.java b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/user/RolesTest.java index 9e128a17c13..59484fbb0ce 100644 --- a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/user/RolesTest.java +++ b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/user/RolesTest.java @@ -6,9 +6,10 @@ import com.yahoo.config.provision.TenantName; import com.yahoo.vespa.hosted.controller.api.role.ApplicationRole; import com.yahoo.vespa.hosted.controller.api.role.Role; import com.yahoo.vespa.hosted.controller.api.role.TenantRole; -import org.junit.Test; +import org.junit.jupiter.api.Test; -import static org.junit.Assert.assertEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; /** * @author jonmv @@ -16,7 +17,7 @@ import static org.junit.Assert.assertEquals; public class RolesTest { @Test - public void testSerialization() { + void testSerialization() { TenantName tenant = TenantName.from("my-tenant"); for (TenantRole role : Roles.tenantRoles(tenant)) assertEquals(role, Roles.toRole(Roles.valueOf(role))); @@ -26,38 +27,48 @@ public class RolesTest { assertEquals(role, Roles.toRole(Roles.valueOf(role))); assertEquals(Role.hostedOperator(), - Roles.toRole("hostedOperator")); + Roles.toRole("hostedOperator")); assertEquals(Role.hostedSupporter(), - Roles.toRole("hostedSupporter")); + Roles.toRole("hostedSupporter")); assertEquals(Role.administrator(tenant), Roles.toRole("my-tenant.administrator")); assertEquals(Role.developer(tenant), Roles.toRole("my-tenant.developer")); assertEquals(Role.reader(tenant), Roles.toRole("my-tenant.reader")); assertEquals(Role.headless(tenant, application), Roles.toRole("my-tenant.my-application.headless")); } - @Test(expected = IllegalArgumentException.class) - public void illegalTenantName() { - Roles.valueOf(Role.developer(TenantName.from("my.tenant"))); + @Test + void illegalTenantName() { + assertThrows(IllegalArgumentException.class, () -> { + Roles.valueOf(Role.developer(TenantName.from("my.tenant"))); + }); } - @Test(expected = IllegalArgumentException.class) - public void illegalApplicationName() { - Roles.valueOf(Role.headless(TenantName.from("my-tenant"), ApplicationName.from("my.app"))); + @Test + void illegalApplicationName() { + assertThrows(IllegalArgumentException.class, () -> { + Roles.valueOf(Role.headless(TenantName.from("my-tenant"), ApplicationName.from("my.app"))); + }); } - @Test(expected = IllegalArgumentException.class) - public void illegalRoleValue() { - Roles.toRole("my-tenant.awesomePerson"); + @Test + void illegalRoleValue() { + assertThrows(IllegalArgumentException.class, () -> { + Roles.toRole("my-tenant.awesomePerson"); + }); } - @Test(expected = IllegalArgumentException.class) - public void illegalCombination() { - Roles.toRole("my-tenant.my-application.tenantOwner"); + @Test + void illegalCombination() { + assertThrows(IllegalArgumentException.class, () -> { + Roles.toRole("my-tenant.my-application.tenantOwner"); + }); } - @Test(expected = IllegalArgumentException.class) - public void illegalValue() { - Roles.toRole("everyone"); + @Test + void illegalValue() { + assertThrows(IllegalArgumentException.class, () -> { + Roles.toRole("everyone"); + }); } } diff --git a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/role/PathGroupTest.java b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/role/PathGroupTest.java index 1923365976e..4a8e9785c88 100644 --- a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/role/PathGroupTest.java +++ b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/role/PathGroupTest.java @@ -1,14 +1,14 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.api.role; -import org.junit.Test; +import org.junit.jupiter.api.Test; import java.util.HashSet; import java.util.Set; import java.util.regex.Pattern; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.fail; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.fail; /** * @author jonmv @@ -17,7 +17,7 @@ import static org.junit.Assert.fail; public class PathGroupTest { @Test - public void uniqueMatches() { + void uniqueMatches() { // Ensure that each path group contains at most one match for any given path, to avoid undefined context extraction. Set checkedAgainstSelf = new HashSet<>(); for (PathGroup group1 : PathGroup.values()) @@ -37,29 +37,29 @@ public class PathGroupTest { // and the other doesn't end with a wildcard matcher ... // and the longest one isn't just one wildcard longer ... // then one is strictly longer than the other, and it's not a match. - if (end < parts1.length && (end == 0 || ! parts2[end - 1].equals("{*}")) && ! parts1[end].equals("{*}")) continue; - if (end < parts2.length && (end == 0 || ! parts1[end - 1].equals("{*}")) && ! parts2[end].equals("{*}")) continue; + if (end < parts1.length && (end == 0 || !parts2[end - 1].equals("{*}")) && !parts1[end].equals("{*}")) continue; + if (end < parts2.length && (end == 0 || !parts1[end - 1].equals("{*}")) && !parts2[end].equals("{*}")) continue; int i; for (i = 0; i < end; i++) if ( ! parts1[i].equals(parts2[i]) - && ! (parts1[i].startsWith("{") && parts1[i].endsWith("}")) - && ! (parts2[i].startsWith("{") && parts2[i].endsWith("}"))) break; + && !(parts1[i].startsWith("{") && parts1[i].endsWith("}")) + && !(parts2[i].startsWith("{") && parts2[i].endsWith("}"))) break; if (i == end) fail("Paths '" + path1 + "' and '" + path2 + "' overlap."); } assertEquals(PathGroup.all().stream().mapToInt(group -> group.pathSpecs.size()).sum(), - checkedAgainstSelf.size()); + checkedAgainstSelf.size()); } @Test - public void contextMatches() { + void contextMatches() { for (PathGroup group : PathGroup.values()) for (String spec : group.pathSpecs) { for (PathGroup.Matcher matcher : PathGroup.Matcher.values()) { if (group.matchers.contains(matcher)) { - if ( ! spec.contains(matcher.pattern)) + if (!spec.contains(matcher.pattern)) fail("Spec '" + spec + "' in '" + group.name() + "' should contain matcher '" + matcher.pattern + "'."); if (spec.replaceFirst(Pattern.quote(matcher.pattern), "").contains(matcher.pattern)) fail("Spec '" + spec + "' in '" + group.name() + "' contains more than one instance of '" + matcher.pattern + "'."); diff --git a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/role/RoleTest.java b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/role/RoleTest.java index 872f74ca1ab..9dac13482e0 100644 --- a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/role/RoleTest.java +++ b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/role/RoleTest.java @@ -4,14 +4,14 @@ package com.yahoo.vespa.hosted.controller.api.role; import com.yahoo.config.provision.ApplicationName; import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.TenantName; -import org.junit.Test; +import org.junit.jupiter.api.Test; import java.net.URI; import java.util.List; import java.util.stream.Stream; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; /** * @author mpolden @@ -23,7 +23,7 @@ public class RoleTest { private static final Enforcer publicCdEnforcer = new Enforcer(SystemName.PublicCd); @Test - public void operator_membership() { + void operator_membership() { Role role = Role.hostedOperator(); // Operator actions @@ -39,7 +39,7 @@ public class RoleTest { } @Test - public void supporter_membership() { + void supporter_membership() { Role role = Role.hostedSupporter(); // No create update or delete @@ -64,11 +64,11 @@ public class RoleTest { } @Test - public void tenant_membership() { + void tenant_membership() { Role role = Role.athenzTenantAdmin(TenantName.from("t1")); assertFalse(mainEnforcer.allows(role, Action.create, URI.create("/not/explicitly/defined"))); - assertFalse("Deny access to operator API", mainEnforcer.allows(role, Action.create, URI.create("/controller/v1/foo"))); - assertFalse("Deny access to other tenant and app", mainEnforcer.allows(role, Action.update, URI.create("/application/v4/tenant/t2/application/a2"))); + assertFalse(mainEnforcer.allows(role, Action.create, URI.create("/controller/v1/foo")), "Deny access to operator API"); + assertFalse(mainEnforcer.allows(role, Action.update, URI.create("/application/v4/tenant/t2/application/a2")), "Deny access to other tenant and app"); assertTrue(mainEnforcer.allows(role, Action.update, URI.create("/application/v4/tenant/t1/application/a1"))); Role publicSystem = Role.athenzTenantAdmin(TenantName.from("t1")); @@ -78,16 +78,16 @@ public class RoleTest { } @Test - public void build_service_membership() { + void build_service_membership() { Role role = Role.buildService(TenantName.from("t1"), ApplicationName.from("a1")); assertFalse(publicEnforcer.allows(role, Action.create, URI.create("/not/explicitly/defined"))); assertFalse(publicEnforcer.allows(role, Action.update, URI.create("/application/v4/tenant/t1/application/a1"))); assertTrue(publicEnforcer.allows(role, Action.create, URI.create("/application/v4/tenant/t1/application/a1/submit"))); - assertFalse("No global read access", publicEnforcer.allows(role, Action.read, URI.create("/controller/v1/foo"))); + assertFalse(publicEnforcer.allows(role, Action.read, URI.create("/controller/v1/foo")), "No global read access"); } @Test - public void new_implications() { + void new_implications() { TenantName tenant1 = TenantName.from("t1"); ApplicationName application1 = ApplicationName.from("a1"); ApplicationName application2 = ApplicationName.from("a2"); @@ -103,7 +103,7 @@ public class RoleTest { } @Test - public void system_flags() { + void system_flags() { URI deployUri = URI.create("/system-flags/v1/deploy"); Action action = Action.update; assertTrue(mainEnforcer.allows(Role.systemFlagsDeployer(), action, deployUri)); @@ -121,7 +121,7 @@ public class RoleTest { } @Test - public void routing() { + void routing() { var tenantUrl = URI.create("/routing/v1/status/tenant/t1"); var applicationUrl = URI.create("/routing/v1/status/tenant/t1/application/a1"); var instanceUrl = URI.create("/routing/v1/status/tenant/t1/application/a1/instance/i1"); @@ -130,8 +130,8 @@ public class RoleTest { for (var url : List.of(tenantUrl, applicationUrl, instanceUrl, deploymentUrl)) { var allowedRole = Role.reader(TenantName.from("t1")); var disallowedRole = Role.reader(TenantName.from("t2")); - assertTrue(allowedRole + " can read " + url, mainEnforcer.allows(allowedRole, Action.read, url)); - assertFalse(disallowedRole + " cannot read " + url, mainEnforcer.allows(disallowedRole, Action.read, url)); + assertTrue(mainEnforcer.allows(allowedRole, Action.read, url), allowedRole + " can read " + url); + assertFalse(mainEnforcer.allows(disallowedRole, Action.read, url), disallowedRole + " cannot read " + url); } // Write @@ -139,15 +139,15 @@ public class RoleTest { var url = URI.create("/routing/v1/inactive/tenant/t1/application/a1/instance/i1/environment/prod/region/us-north-1"); var allowedRole = Role.developer(TenantName.from("t1")); var disallowedRole = Role.developer(TenantName.from("t2")); - assertTrue(allowedRole + " can override status at " + url, mainEnforcer.allows(allowedRole, Action.create, url)); - assertTrue(allowedRole + " can clear status at " + url, mainEnforcer.allows(allowedRole, Action.delete, url)); - assertFalse(disallowedRole + " cannot override status at " + url, mainEnforcer.allows(disallowedRole, Action.create, url)); - assertFalse(disallowedRole + " cannot clear status at " + url, mainEnforcer.allows(disallowedRole, Action.delete, url)); + assertTrue(mainEnforcer.allows(allowedRole, Action.create, url), allowedRole + " can override status at " + url); + assertTrue(mainEnforcer.allows(allowedRole, Action.delete, url), allowedRole + " can clear status at " + url); + assertFalse(mainEnforcer.allows(disallowedRole, Action.create, url), disallowedRole + " cannot override status at " + url); + assertFalse(mainEnforcer.allows(disallowedRole, Action.delete, url), disallowedRole + " cannot clear status at " + url); } } @Test - public void payment_instrument() { + void payment_instrument() { URI paymentInstrumentUri = URI.create("/billing/v1/tenant/t1/instrument/foobar"); URI tenantPaymentInstrumentUri = URI.create("/billing/v1/tenant/t1/instrument"); URI tokenUri = URI.create("/billing/v1/tenant/t1/token"); @@ -172,7 +172,7 @@ public class RoleTest { } @Test - public void billing_tenant() { + void billing_tenant() { URI billing = URI.create("/billing/v1/tenant/t1/billing"); Role user = Role.reader(TenantName.from("t1")); @@ -189,7 +189,7 @@ public class RoleTest { } @Test - public void billing_test() { + void billing_test() { var tester = new EnforcerTester(publicEnforcer); var accountant = Role.hostedAccountant(); @@ -302,12 +302,12 @@ public class RoleTest { allowed.forEach(action -> { var msg = String.format("%s should be allowed to %s on %s", role, action, resource); - assertTrue(msg, enforcer.allows(role, action, resource)); + assertTrue(enforcer.allows(role, action, resource), msg); }); Action.all().stream().filter(a -> ! allowed.contains(a)).forEach(action -> { var msg = String.format("%s should not be allowed to %s on %s", role, action, resource); - assertFalse(msg, enforcer.allows(role, action, resource)); + assertFalse(enforcer.allows(role, action, resource), msg); }); return this; diff --git a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchiveTest.java b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchiveTest.java index ce2746f92e0..c40788ecb3f 100644 --- a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchiveTest.java +++ b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchiveTest.java @@ -16,10 +16,8 @@ import com.yahoo.vespa.flags.FlagId; import com.yahoo.vespa.flags.RawFlag; import com.yahoo.vespa.flags.json.FlagData; import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneRegistry; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.ExpectedException; -import org.junit.rules.TemporaryFolder; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; import java.io.BufferedInputStream; import java.io.BufferedOutputStream; @@ -37,9 +35,10 @@ import java.util.Map; import java.util.Set; import static java.util.stream.Collectors.toList; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -53,12 +52,8 @@ public class SystemFlagsDataArchiveTest { private static final FlagId MY_TEST_FLAG = new FlagId("my-test-flag"); private static final FlagId FLAG_WITH_EMPTY_DATA = new FlagId("flag-with-empty-data"); - @Rule - public final TemporaryFolder temporaryFolder = new TemporaryFolder(); - - @SuppressWarnings("deprecation") - @Rule - public final ExpectedException expectedException = ExpectedException.none(); + @TempDir + public File temporaryFolder; private static final FlagsTarget mainControllerTarget = FlagsTarget.forController(SYSTEM); private static final FlagsTarget cdControllerTarget = FlagsTarget.forController(SystemName.cd); @@ -75,8 +70,8 @@ public class SystemFlagsDataArchiveTest { } @Test - public void can_serialize_and_deserialize_archive() throws IOException { - File tempFile = temporaryFolder.newFile("serialized-flags-archive"); + void can_serialize_and_deserialize_archive() throws IOException { + File tempFile = File.createTempFile("serialized-flags-archive", null, temporaryFolder); try (OutputStream out = new BufferedOutputStream(new FileOutputStream(tempFile))) { var archive = SystemFlagsDataArchive.fromDirectory(Paths.get("src/test/resources/system-flags/")); archive.toZip(out); @@ -88,26 +83,27 @@ public class SystemFlagsDataArchiveTest { } @Test - public void retrieves_correct_flag_data_for_target() { + void retrieves_correct_flag_data_for_target() { var archive = SystemFlagsDataArchive.fromDirectory(Paths.get("src/test/resources/system-flags/")); assertArchiveReturnsCorrectTestFlagDataForTarget(archive); } @Test - public void supports_multi_level_flags_directory() { + void supports_multi_level_flags_directory() { var archive = SystemFlagsDataArchive.fromDirectory(Paths.get("src/test/resources/system-flags-multi-level/")); assertFlagDataHasValue(archive, MY_TEST_FLAG, mainControllerTarget, "default"); } @Test - public void duplicated_flagdata_is_detected() { - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("contains redundant flag data for id 'my-test-flag' already set in another directory!"); - var archive = SystemFlagsDataArchive.fromDirectory(Paths.get("src/test/resources/system-flags-multi-level-with-duplicated-flagdata/")); + void duplicated_flagdata_is_detected() { + Throwable exception = assertThrows(IllegalArgumentException.class, () -> { + var archive = SystemFlagsDataArchive.fromDirectory(Paths.get("src/test/resources/system-flags-multi-level-with-duplicated-flagdata/")); + }); + assertTrue(exception.getMessage().contains("contains redundant flag data for id 'my-test-flag' already set in another directory!")); } @Test - public void empty_files_are_handled_as_no_flag_data_for_target() { + void empty_files_are_handled_as_no_flag_data_for_target() { var archive = SystemFlagsDataArchive.fromDirectory(Paths.get("src/test/resources/system-flags/")); assertNoFlagData(archive, FLAG_WITH_EMPTY_DATA, mainControllerTarget); assertFlagDataHasValue(archive, FLAG_WITH_EMPTY_DATA, prodUsWestCfgTarget, "main.prod.us-west-1"); @@ -116,43 +112,45 @@ public class SystemFlagsDataArchiveTest { } @Test - public void throws_exception_on_non_json_file() { - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("Only JSON files are allowed in 'flags/' directory (found 'flags/my-test-flag/file-name-without-dot-json')"); - SystemFlagsDataArchive.fromDirectory(Paths.get("src/test/resources/system-flags-with-invalid-file-name/")); + void throws_exception_on_non_json_file() { + Throwable exception = assertThrows(IllegalArgumentException.class, () -> { + SystemFlagsDataArchive.fromDirectory(Paths.get("src/test/resources/system-flags-with-invalid-file-name/")); + }); + assertTrue(exception.getMessage().contains("Only JSON files are allowed in 'flags/' directory (found 'flags/my-test-flag/file-name-without-dot-json')")); } @Test - public void throws_exception_on_unknown_file() { - SystemFlagsDataArchive archive = SystemFlagsDataArchive.fromDirectory(Paths.get("src/test/resources/system-flags-with-unknown-file-name/")); - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("Unknown flag file: flags/my-test-flag/main.prod.unknown-region.json"); - archive.validateAllFilesAreForTargets(SystemName.main, Set.of(mainControllerTarget, prodUsWestCfgTarget)); + void throws_exception_on_unknown_file() { + Throwable exception = assertThrows(IllegalArgumentException.class, () -> { + SystemFlagsDataArchive archive = SystemFlagsDataArchive.fromDirectory(Paths.get("src/test/resources/system-flags-with-unknown-file-name/")); + archive.validateAllFilesAreForTargets(SystemName.main, Set.of(mainControllerTarget, prodUsWestCfgTarget)); + }); + assertTrue(exception.getMessage().contains("Unknown flag file: flags/my-test-flag/main.prod.unknown-region.json")); } @Test - public void throws_exception_on_unknown_region() { - Path directory = Paths.get("src/test/resources/system-flags-with-unknown-file-name/"); - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage( - "Environment or zone in filename 'main.prod.unknown-region.json' does not exist"); - SystemFlagsDataArchive.fromDirectoryAndSystem(directory, createZoneRegistryMock()); + void throws_exception_on_unknown_region() { + Throwable exception = assertThrows(IllegalArgumentException.class, () -> { + Path directory = Paths.get("src/test/resources/system-flags-with-unknown-file-name/"); + SystemFlagsDataArchive.fromDirectoryAndSystem(directory, createZoneRegistryMock()); + }); + assertTrue(exception.getMessage().contains("Environment or zone in filename 'main.prod.unknown-region.json' does not exist")); } @Test - public void throws_on_unknown_field() { - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage( - "flags/my-test-flag/main.prod.us-west-1.json contains unknown non-comment fields: after removing any comment fields the JSON is:\n" + + void throws_on_unknown_field() { + Throwable exception = assertThrows(IllegalArgumentException.class, () -> { + SystemFlagsDataArchive.fromDirectory(Paths.get("src/test/resources/system-flags-with-unknown-field-name/")); + }); + assertTrue(exception.getMessage().contains("flags/my-test-flag/main.prod.us-west-1.json contains unknown non-comment fields: after removing any comment fields the JSON is:\n" + " {\"id\":\"my-test-flag\",\"rules\":[{\"condition\":[{\"type\":\"whitelist\",\"dimension\":\"hostname\",\"values\":[\"foo.com\"]}],\"value\":\"default\"}]}\n" + "but deserializing this ended up with a JSON that are missing some of the fields:\n" + " {\"id\":\"my-test-flag\",\"rules\":[{\"value\":\"default\"}]}\n" + - "See https://git.ouroath.com/vespa/hosted-feature-flags for more info on the JSON syntax"); - SystemFlagsDataArchive.fromDirectory(Paths.get("src/test/resources/system-flags-with-unknown-field-name/")); + "See https://git.ouroath.com/vespa/hosted-feature-flags for more info on the JSON syntax")); } @Test - public void remove_comments() { + void remove_comments() { assertTrue(JSON.equals("{\n" + " \"a\": {\n" + " \"b\": 1\n" + @@ -184,7 +182,7 @@ public class SystemFlagsDataArchiveTest { } @Test - public void normalize_json_fail_on_invalid_application() { + void normalize_json_fail_on_invalid_application() { try { SystemFlagsDataArchive.normalizeJson("{\n" + " \"id\": \"foo\",\n" + @@ -208,7 +206,7 @@ public class SystemFlagsDataArchiveTest { } @Test - public void normalize_json_fail_on_invalid_node_type() { + void normalize_json_fail_on_invalid_node_type() { try { SystemFlagsDataArchive.normalizeJson("{\n" + " \"id\": \"foo\",\n" + @@ -232,7 +230,7 @@ public class SystemFlagsDataArchiveTest { } @Test - public void normalize_json_fail_on_invalid_email() { + void normalize_json_fail_on_invalid_email() { try { SystemFlagsDataArchive.normalizeJson("{\n" + " \"id\": \"foo\",\n" + @@ -256,7 +254,7 @@ public class SystemFlagsDataArchiveTest { } @Test - public void normalize_json_fail_on_invalid_tenant_id() { + void normalize_json_fail_on_invalid_tenant_id() { try { SystemFlagsDataArchive.normalizeJson("{\n" + " \"id\": \"foo\",\n" + @@ -280,7 +278,7 @@ public class SystemFlagsDataArchiveTest { } @Test - public void ignores_files_not_related_to_specified_system_definition() { + void ignores_files_not_related_to_specified_system_definition() { ZoneRegistry registry = createZoneRegistryMock(); Path testDirectory = Paths.get("src/test/resources/system-flags-for-multiple-systems/"); var archive = SystemFlagsDataArchive.fromDirectoryAndSystem(testDirectory, registry); -- cgit v1.2.3