From cd91de7f690e8638e52bb4dbff9ec94b97ff02cd Mon Sep 17 00:00:00 2001 From: labkey-danield Date: Fri, 15 May 2026 15:52:08 -0700 Subject: [PATCH 1/2] Move test into SampleTypeNameExpressionTest.testInputsExpression. Rewrote helper methods to be more flexible and not make as many assumptions of test data. Use regular expression for the validation of sample names generated from the NameExpression. --- .../tests/SampleTypeNameExpressionTest.java | 162 +++++++++++++++--- 1 file changed, 140 insertions(+), 22 deletions(-) diff --git a/src/org/labkey/test/tests/SampleTypeNameExpressionTest.java b/src/org/labkey/test/tests/SampleTypeNameExpressionTest.java index 1835704bd0..1abd90d423 100644 --- a/src/org/labkey/test/tests/SampleTypeNameExpressionTest.java +++ b/src/org/labkey/test/tests/SampleTypeNameExpressionTest.java @@ -34,6 +34,7 @@ import org.labkey.test.params.FieldDefinition; import org.labkey.test.params.FieldDefinition.ColumnType; import org.labkey.test.params.FieldInfo; +import org.labkey.test.params.experiment.DataClassDefinition; import org.labkey.test.params.experiment.SampleTypeDefinition; import org.labkey.test.util.AuditLogHelper; import org.labkey.test.util.DataRegionTable; @@ -58,12 +59,12 @@ import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.regex.Pattern; import static org.hamcrest.CoreMatchers.hasItems; import static org.hamcrest.CoreMatchers.startsWith; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; import static org.labkey.test.params.FieldDefinition.DOMAIN_TRICKY_CHARACTERS; import static org.labkey.test.util.EscapeUtil.escapeForNameExpression; @@ -514,53 +515,134 @@ public void testWithTrickyFieldNames() throws IOException, CommandException } + // Field used in the testInputsExpression test. + private static final String SHARED_FIELD_NAME = "FieldB"; + @Test - public void testInputsExpression() + public void testInputsExpression() throws IOException, CommandException { + verifyNames( "InputsExpressionTest", - "Name\tFieldB\tMaterialInputs/InputsExpressionTest", + "Name\t" + SHARED_FIELD_NAME + "\tMaterialInputs/InputsExpressionTest", "${Inputs:first:defaultValue('" + DEFAULT_SAMPLE_PARENT_VALUE + "')}_${batchRandomId}", null, "Pat"); - final String urlLikeDefaultVal = "a+b%c#d"; verifyNames( "InputsExpressionTest2", - "Name\tFieldB\tMaterialInputs/InputsExpressionTest2", + "Name\t" + SHARED_FIELD_NAME + "\tMaterialInputs/InputsExpressionTest2", "${Inputs:first:defaultValue('" + urlLikeDefaultVal + "')}_${batchRandomId}", null, "Fed", true, urlLikeDefaultVal); verifyNames( "Inputs/ExpressionTest2", - "Name\tFieldB\tMaterialInputs/Inputs$SExpressionTest2", + "Name\t" + SHARED_FIELD_NAME + "\tMaterialInputs/Inputs$SExpressionTest2", "${Inputs:defaultValue('" + DEFAULT_SAMPLE_PARENT_VALUE + "')}_${batchRandomId}", null, "Bat", false); verifyNames( "Inputs/WithDataTypeExpression", - "Name\tFieldB\tMaterialInputs/Inputs/WithDataTypeExpression", + "Name\t" + SHARED_FIELD_NAME + "\tMaterialInputs/Inputs/WithDataTypeExpression", "${Inputs/Inputs$SWithDataTypeExpression:first:defaultValue('" + DEFAULT_SAMPLE_PARENT_VALUE + "')}_${batchRandomId}", null, "Red"); verifyNames( "Inputs/WithDataTypeExpression2", - "Name\tFieldB\tMaterialInputs/Inputs$SWithDataTypeExpression2", + "Name\t" + SHARED_FIELD_NAME + "\tMaterialInputs/Inputs$SWithDataTypeExpression2", "${Inputs/Inputs$SWithDataTypeExpression2:defaultValue('" + DEFAULT_SAMPLE_PARENT_VALUE + "')}_${batchRandomId}", null, "Ted", false); verifyNames( "Material/WithDataTypeExpression", - "Name\tFieldB\tMaterialInputs/Material/WithDataTypeExpression", + "Name\t" + SHARED_FIELD_NAME + "\tMaterialInputs/Material/WithDataTypeExpression", "${MaterialInputs/Material$SWithDataTypeExpression:first:defaultValue('" + DEFAULT_SAMPLE_PARENT_VALUE + "')}_${batchRandomId}", null, "Ned"); verifyNames( "Material/WithDataTypeExpression2", - "Name\tFieldB\tMaterialInputs/Material$SWithDataTypeExpression2", + "Name\t" + SHARED_FIELD_NAME + "\tMaterialInputs/Material$SWithDataTypeExpression2", "${MaterialInputs/Material$SWithDataTypeExpression2:defaultValue('" + DEFAULT_SAMPLE_PARENT_VALUE + "')}_${batchRandomId}", null, "Med", false); + // Test that a name expression using a generic "Inputs" and a field name can resolve the field name in Material and Data classes. + log("Test a name expression that uses \"Inputs/SomeFieldName\"."); + + log("Create a DataClass to use as a parent."); + final String dataParentClass = "InputsDataParentFieldExpression"; + DataClassDefinition dpDef = new DataClassDefinition(dataParentClass); + dpDef.setFields(List.of(new FieldDefinition(SHARED_FIELD_NAME, ColumnType.String))); + TestDataGenerator dpGen = dpDef.create(createDefaultConnection(), getProjectName()); + dpGen.addCustomRow(Map.of("Name", "DS-1", + SHARED_FIELD_NAME, "ABC")); + dpGen.addCustomRow(Map.of("Name", "DS-2", + SHARED_FIELD_NAME, "DEF")); + dpGen.insertRows(createDefaultConnection(), dpGen.getRows()); + + final String nameExpression = "Gen-${Inputs/" + SHARED_FIELD_NAME + ":defaultValue('none')}-${genId}"; + + StringBuilder data = new StringBuilder(); + data.append(SHARED_FIELD_NAME); + data.append("\tDataInputs/InputsDataParentFieldExpression\n"); + data.append("a\tDS-1\n"); + data.append("b\tDS-2\n"); + data.append("c\tDS-1, DS-2\n"); + data.append("d\n"); + + goToProjectHome(); + final String sampleTypeName = "InputsExpressionTestData"; + createSampleTypeForNameValidation(sampleTypeName, + nameExpression, null, data.toString()); + + DataRegionTable materialTable = new DataRegionTable("Material", this); + List actualNames = materialTable.getColumnDataAsText("Name"); + + List dataSourceParentNames = List.of( + Pattern.quote("Gen-none-") + ".*", + Pattern.quote("Gen-[ABC, DEF]-") + ".*", + Pattern.quote("Gen-DEF-") + ".*", + Pattern.quote("Gen-ABC-") + ".*" + ); + validateNamesGenerated(actualNames, dataSourceParentNames); + + data = new StringBuilder(); + data.append("Name\t"); + data.append(SHARED_FIELD_NAME); + data.append("\n"); + data.append("S-1\tUVW\n"); + data.append("S-2\tXYZ\n"); + + goToProjectHome(); + final String parentSampleType = "InputsMaterialParentFieldExpression"; + createSampleTypeForNameValidation(parentSampleType, + "S-${genId}", null, data.toString()); + + data = new StringBuilder(); + data.append(SHARED_FIELD_NAME); + data.append("\tMaterialInputs/"); + data.append(parentSampleType); + data.append("\n"); + data.append("i\tS-1\n"); + data.append("j\tS-2\n"); + data.append("k\tS-1, S-2\n"); + data.append("l\n"); + + goToProjectHome(); + SampleTypeHelper sampleHelper = new SampleTypeHelper(this); + sampleHelper.goToSampleType(sampleTypeName).bulkImport(data.toString()); + + List sampleTypeParentNames = new ArrayList<>(); + sampleTypeParentNames.add(Pattern.quote("Gen-none-") + ".*"); + sampleTypeParentNames.add(Pattern.quote("Gen-[UVW, XYZ]-") + ".*"); + sampleTypeParentNames.add(Pattern.quote("Gen-XYZ-") + ".*"); + sampleTypeParentNames.add(Pattern.quote("Gen-UVW-") + ".*"); + sampleTypeParentNames.addAll(dataSourceParentNames); + + materialTable = new DataRegionTable("Material", this); + actualNames = materialTable.getColumnDataAsText("Name"); + + validateNamesGenerated(actualNames, sampleTypeParentNames); + } @Test @@ -663,30 +745,66 @@ private void verifyNames(String sampleTypeName, String header, String nameExpres // Name generated and uses defaultValue('SS') "\tb\t\n"; + createSampleTypeForNameValidation(sampleTypeName, nameExpression, currentTypeAlias, data); + + DataRegionTable materialTable = new DataRegionTable("Material", this); + List actualNames = materialTable.getColumnDataAsText("Name"); + + List expectedNames = new ArrayList<>(); + + expectedNames.add(Pattern.quote(defaultValue + "_") + ".*"); + + String batchRandomId = actualNames.getFirst().substring(actualNames.getFirst().lastIndexOf("_") + 1); + + expectedNames.add(Pattern.quote(name2 + "_" + batchRandomId)); + + String pattern = useFirst ? name1 : "[" + name1 + ", " + name2 + "]"; + pattern = pattern + "_" + batchRandomId; + expectedNames.add(Pattern.quote(pattern)); + + expectedNames.add(Pattern.quote(name2)); + + expectedNames.add(Pattern.quote(name1)); + + validateNamesGenerated(actualNames, expectedNames); + } + + private void createSampleTypeForNameValidation(String sampleTypeName, String nameExpression, @Nullable String currentTypeAlias, String data) + { + log(String.format("Create a sample type %s with a name expression of %s and populate it using the name expression.", + sampleTypeName, nameExpression)); + SampleTypeHelper sampleHelper = new SampleTypeHelper(this); SampleTypeDefinition definition = new SampleTypeDefinition(sampleTypeName) .setNameExpression(nameExpression); if (currentTypeAlias != null) definition = definition.setParentAliases(Map.of(currentTypeAlias, "(Current Sample Type)")); - definition = definition.setFields(List.of(new FieldDefinition("FieldB", ColumnType.String))); + definition = definition.setFields(List.of(new FieldDefinition(SHARED_FIELD_NAME, ColumnType.String))); sampleHelper.createSampleType(definition, data); assertTextPresent(nameExpression); - DataRegionTable materialTable = new DataRegionTable("Material", this); - List names = materialTable.getColumnDataAsText("Name"); - - // The next two lines assume the name expression has specific values in specific locations, and as far as I - // can tell, that is how the tests are written. - assertTrue("First name (" + names.get(0) + ") expected to start with " + defaultValue + "_ but it did not", names.get(0).startsWith(defaultValue + "_")); - String batchRandomId = names.get(0).substring(names.get(0).lastIndexOf("_") + 1); + } - assertEquals("Second name not as expected", name2 + "_" + batchRandomId, names.get(1)); + private void validateNamesGenerated(List actualNames, List expectedNames) + { - assertEquals("Third name not as expected", (useFirst ? name1 : ("[" + name1 + ", " + name2 + "]")) + "_" + batchRandomId, names.get(2)); + if (checker().verifyEquals("Number of samples not as expected.", + expectedNames.size(), actualNames.size())) + { + for (int i = 0; i < expectedNames.size(); i++) + { + String expectedPattern = expectedNames.get(i); + String actual = actualNames.get(i); + + checker().verifyTrue(String.format( + "Name at index %d did not match. Expected pattern: [%s] Actual: [%s]", + i, expectedPattern, actual), + Pattern.matches(expectedPattern, actual)); + } + } - assertEquals("Fourth name not as expected", name2, names.get(3)); - assertEquals("Fifth name not as expected", name1, names.get(4)); + checker().screenShotIfNewError("Generated_Names_Error"); } /** From 000a4ebfc6061975f049b390aabbd7fab350487c Mon Sep 17 00:00:00 2001 From: labkey-danield Date: Fri, 15 May 2026 16:33:53 -0700 Subject: [PATCH 2/2] Use Claude to clean up SampleTypeNameExpressionTest.validateNamesGenerated. --- .../tests/SampleTypeNameExpressionTest.java | 46 ++++++++++++++----- 1 file changed, 34 insertions(+), 12 deletions(-) diff --git a/src/org/labkey/test/tests/SampleTypeNameExpressionTest.java b/src/org/labkey/test/tests/SampleTypeNameExpressionTest.java index 1abd90d423..e98c7e09dc 100644 --- a/src/org/labkey/test/tests/SampleTypeNameExpressionTest.java +++ b/src/org/labkey/test/tests/SampleTypeNameExpressionTest.java @@ -59,6 +59,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Optional; import java.util.regex.Pattern; import static org.hamcrest.CoreMatchers.hasItems; @@ -786,24 +787,45 @@ private void createSampleTypeForNameValidation(String sampleTypeName, String nam } + /** + *

+ * Validate that every name in {@code actualNames} matches one of the regex patterns in + * {@code expectedNames}, using a multiset diff: every expected pattern must match at least + * one actual name, every actual name must be matched by some expected pattern, and the + * counts must be equal. Order is intentionally NOT verified. + *

+ *

+ * On a mismatch the failure reports both the patterns that found no match and the actual + * names that found no pattern, so a "missing row" or "extra row" failure is immediately + * diagnosable instead of just a "wrong count" error. + *

+ *

+ * This method was generated by Claude. + *

+ */ private void validateNamesGenerated(List actualNames, List expectedNames) { + checker().verifyEquals("Number of samples not as expected.", + expectedNames.size(), actualNames.size()); - if (checker().verifyEquals("Number of samples not as expected.", - expectedNames.size(), actualNames.size())) + List unmatchedActuals = new ArrayList<>(actualNames); + List unmatchedPatterns = new ArrayList<>(); + for (String pattern : expectedNames) { - for (int i = 0; i < expectedNames.size(); i++) - { - String expectedPattern = expectedNames.get(i); - String actual = actualNames.get(i); - - checker().verifyTrue(String.format( - "Name at index %d did not match. Expected pattern: [%s] Actual: [%s]", - i, expectedPattern, actual), - Pattern.matches(expectedPattern, actual)); - } + Optional match = unmatchedActuals.stream() + .filter(a -> Pattern.matches(pattern, a)) + .findFirst(); + if (match.isPresent()) + unmatchedActuals.remove(match.get()); + else + unmatchedPatterns.add(pattern); } + checker().verifyTrue("Expected patterns with no matching actual name: " + unmatchedPatterns, + unmatchedPatterns.isEmpty()); + checker().verifyTrue("Actual names not matching any expected pattern (extras): " + unmatchedActuals, + unmatchedActuals.isEmpty()); + checker().screenShotIfNewError("Generated_Names_Error"); }