>From Michael Blow <[email protected]>: Michael Blow has submitted this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21664?usp=email )
Change subject: [NO ISSUE][TEST] Allow multiple test categories ...................................................................... [NO ISSUE][TEST] Allow multiple test categories The category attribute of a test-case is single-valued and its only value is "slow", so a case can be in at most one category. Two axes already exist in practice, and a case may need to sit on both. - make category a whitespace-separated list of category-enum, so a case can declare category="slow requires-stable-topology" - add requires-stable-topology and requires-default-frame-size, and say in the schema what a category means: something a test needs from the suite running it, so a suite that cannot provide it can leave the test out. It is up to each consumer to decide which categories it honours; the builder itself still honours only "slow" - add TestCaseContext.hasCategory so a consumer need not reach through getTestCase() - reject an unrecognized category. JAXB unmarshals an unknown list item to a null entry rather than failing, so a misspelled category would otherwise stop excluding the test it was written to exclude, and look correct doing it. Schema validation would also catch this, but the suite documents cannot be validated as they stand: testgroups.xml has a test-group with no name, and name is use="required". No existing suite declares a category, so nothing changes for callers other than the getCategory signature. Declaring hyracks-util brings in @AiProvenance, which is the tree's convention for AI-authored Java. It is @Retention(SOURCE) and so leaves no trace for dependency:analyze, hence the usedDependency entry. Ext-ref: MB-68099 Co-Authored-By: Claude Opus 5 <[email protected]> Change-Id: I2388a943a987b5fbffecfe7984f5462aaf229489 Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21664 Tested-by: Jenkins <[email protected]> Reviewed-by: Ian Maxon <[email protected]> Reviewed-by: Michael Blow <[email protected]> Reviewed-by: Hussain Towaileb <[email protected]> Integration-Tests: Jenkins <[email protected]> --- M asterixdb/asterix-test-framework/pom.xml M asterixdb/asterix-test-framework/src/main/java/org/apache/asterix/testframework/context/TestCaseContext.java M asterixdb/asterix-test-framework/src/main/resources/Catalog.xsd 3 files changed, 73 insertions(+), 5 deletions(-) Approvals: Michael Blow: Looks good to me, but someone else must approve Hussain Towaileb: Looks good to me, approved Ian Maxon: Looks good to me, approved Jenkins: Verified; Verified diff --git a/asterixdb/asterix-test-framework/pom.xml b/asterixdb/asterix-test-framework/pom.xml index c28eb48..e2a0f2e 100644 --- a/asterixdb/asterix-test-framework/pom.xml +++ b/asterixdb/asterix-test-framework/pom.xml @@ -59,6 +59,8 @@ <usedDependency>com.sun.xml.bind:jaxb-core</usedDependency> <usedDependency>com.sun.xml.bind:jaxb-impl</usedDependency> <usedDependency>com.sun.activation:javax.activation</usedDependency> + <!-- @AiProvenance is @Retention(SOURCE), so it leaves no trace for dependency:analyze --> + <usedDependency>org.apache.hyracks:hyracks-util</usedDependency> </usedDependencies> </configuration> </plugin> @@ -90,6 +92,10 @@ <artifactId>jaxb-api</artifactId> </dependency> <dependency> + <groupId>org.apache.hyracks</groupId> + <artifactId>hyracks-util</artifactId> + </dependency> + <dependency> <groupId>org.apache.logging.log4j</groupId> <artifactId>log4j-api</artifactId> </dependency> diff --git a/asterixdb/asterix-test-framework/src/main/java/org/apache/asterix/testframework/context/TestCaseContext.java b/asterixdb/asterix-test-framework/src/main/java/org/apache/asterix/testframework/context/TestCaseContext.java index 6e458a4..4103286 100644 --- a/asterixdb/asterix-test-framework/src/main/java/org/apache/asterix/testframework/context/TestCaseContext.java +++ b/asterixdb/asterix-test-framework/src/main/java/org/apache/asterix/testframework/context/TestCaseContext.java @@ -18,6 +18,8 @@ */ package org.apache.asterix.testframework.context; +import static java.util.stream.Collectors.joining; + import java.io.File; import java.io.IOException; import java.util.ArrayList; @@ -27,6 +29,7 @@ import java.util.List; import java.util.Map; import java.util.regex.Pattern; +import java.util.stream.Stream; import org.apache.asterix.testframework.template.TemplateHelper; import org.apache.asterix.testframework.xml.CategoryEnum; @@ -35,6 +38,7 @@ import org.apache.asterix.testframework.xml.TestGroup; import org.apache.asterix.testframework.xml.TestSuite; import org.apache.asterix.testframework.xml.TestSuiteParser; +import org.apache.hyracks.util.annotations.AiProvenance; public class TestCaseContext { @@ -74,6 +78,16 @@ return testCase; } + /** + * @return whether this test case declares {@code category} in the {@code category} attribute of its test-case + * element. A category names something the test needs from the suite running it, so a suite that cannot + * provide it can leave the test out. + */ + @AiProvenance(agent = AiProvenance.Agent.CLAUDE_OPUS_5, tool = AiProvenance.Tool.CLAUDE_CODE_UI) + public boolean hasCategory(CategoryEnum category) { + return testCase.getCategory().contains(category); + } + public int getRepeat() { return testCase.getRepeat().intValue(); } @@ -219,10 +233,19 @@ } } + @AiProvenance(agent = AiProvenance.Agent.CLAUDE_OPUS_5, tool = AiProvenance.Tool.CLAUDE_CODE_UI, contributionKind = AiProvenance.ContributionKind.REFACTORED, notes = "category is a list") private void addContexts(File tsRoot, TestSuite ts, List<TestGroup> tgPath, List<TestCaseContext> tccs) { TestGroup tg = tgPath.get(tgPath.size() - 1); for (TestCase tc : tg.getTestCase()) { - if (doSlow || tc.getCategory() != CategoryEnum.SLOW) { + // an unrecognized category unmarshals to a null list entry rather than failing, and a + // category nothing matches silently stops excluding the test it was meant to exclude + if (tc.getCategory().contains(null)) { + throw new IllegalArgumentException( + "test case " + tc.getFilePath() + " (" + tc.getCompilationUnit().get(0).getName() + + ") declares an unrecognized category; " + "valid categories are " + + Stream.of(CategoryEnum.values()).map(CategoryEnum::value).collect(joining(", "))); + } + if (doSlow || !tc.getCategory().contains(CategoryEnum.SLOW)) { boolean matches = false; if (re != null) { // Check all compilation units for matching diff --git a/asterixdb/asterix-test-framework/src/main/resources/Catalog.xsd b/asterixdb/asterix-test-framework/src/main/resources/Catalog.xsd index 303a4d7..f657b41 100644 --- a/asterixdb/asterix-test-framework/src/main/resources/Catalog.xsd +++ b/asterixdb/asterix-test-framework/src/main/resources/Catalog.xsd @@ -212,23 +212,62 @@ <!-- the QueryFileExtension --> <xs:attribute name="FilePath" type="test:SimplifiedRelativeFilePath" use="required"/> - <xs:attribute name="category" type="test:category-enum"/> + <xs:attribute name="category" type="test:category-list"/> <xs:attribute name="repeat" type="xs:positiveInteger" default="1" /> <xs:attribute name="check-warnings" type="xs:boolean" default="false"/> </xs:complexType> + <!-- category-list type --> + <!-- A whitespace-separated list of the categories a test is in. --> + + <xs:simpleType name="category-list"> + <xs:annotation> + <xs:documentation> + The categories a test case is in, whitespace-separated. A test can be in more than + one, e.g. category="slow requires-stable-topology". + </xs:documentation> + </xs:annotation> + + <xs:list itemType="test:category-enum"/> + </xs:simpleType> + <!-- category-enum type --> - <!-- Identify which category of test this is. Currently only "slow". --> + <!-- Identify which category of test this is. --> <xs:simpleType name="category-enum"> <xs:annotation> <xs:documentation> - Identify the category of test, for limiting when it is run. + Identify a category of test, for limiting when it is run. A category names something + the test needs from the suite running it, so that a suite which cannot provide it can + leave the test out; it is up to each consumer to decide which categories it honours. </xs:documentation> </xs:annotation> <xs:restriction base="xs:string"> - <xs:enumeration value="slow"/> + <xs:enumeration value="slow"> + <xs:annotation> + <xs:documentation> + Runs long enough that it is skipped unless -DrunSlowAQLTests=true. + </xs:documentation> + </xs:annotation> + </xs:enumeration> + <xs:enumeration value="requires-stable-topology"> + <xs:annotation> + <xs:documentation> + Asserts something that is a function of the cluster topology -- an exact + optimizer plan, say -- and so cannot run in a suite that changes the topology + underneath it. + </xs:documentation> + </xs:annotation> + </xs:enumeration> + <xs:enumeration value="requires-default-frame-size"> + <xs:annotation> + <xs:documentation> + Returns incorrect results at a reduced frame size, so it cannot run in a suite + that configures one. + </xs:documentation> + </xs:annotation> + </xs:enumeration> </xs:restriction> </xs:simpleType> -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21664?usp=email To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: asterixdb Gerrit-Branch: trinity Gerrit-Change-Id: I2388a943a987b5fbffecfe7984f5462aaf229489 Gerrit-Change-Number: 21664 Gerrit-PatchSet: 5 Gerrit-Owner: Michael Blow <[email protected]> Gerrit-Reviewer: Hussain Towaileb <[email protected]> Gerrit-Reviewer: Ian Maxon <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Michael Blow <[email protected]> Gerrit-Reviewer: Murtadha Hubail Gerrit-CC: Anon. E. Moose #1000171
