Skip to content

Fix tricky field name test failures #2470

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
labkey-tchad merged 5 commits into from
May 30, 2025
Merged

Fix tricky field name test failures #2470

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
826b9bf
Fix field reference in CustomizeViewTest
labkey-tchad May 30, 2025
43e05fa
Fix TSV format
labkey-tchad May 30, 2025
ff8823c
Remove random names from GridPanelTest, they cause too much trouble
labkey-tchad May 30, 2025
a07478c
Don't include wide character in random field names
labkey-tchad May 30, 2025
0112ce3
Revert wrong and out-of-scope fix
labkey-tchad May 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/org/labkey/test/tests/CustomizeViewTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,11 +305,11 @@ public void saveFilterTest()
String value = "J";
String[] viewNames = {TRICKY_CHARACTERS + "view", "AAC", "aaa", "aad", "zzz"};

setColumns(fieldKey);
setColumns(LAST_NAME_COLUMN);
for(String name : viewNames)
{
_customizeViewsHelper.openCustomizeViewPanel();
_customizeViewsHelper.addFilter(fieldKey, fieldKey, op, value);
_customizeViewsHelper.addFilter(new String[]{fieldKey}, fieldKey, op, value);
_customizeViewsHelper.saveCustomView(name);
}

Expand Down
15 changes: 8 additions & 7 deletions src/org/labkey/test/tests/component/GridPanelTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.labkey.test.components.ui.search.FilterExpressionPanel;
import org.labkey.test.components.ui.search.FilterFacetedPanel;
import org.labkey.test.params.FieldDefinition;
import org.labkey.test.params.FieldKey;
import org.labkey.test.params.experiment.SampleTypeDefinition;
import org.labkey.test.util.DataRegionTable;
import org.labkey.test.util.SampleTypeHelper;
Expand Down Expand Up @@ -63,11 +64,11 @@ public class GridPanelTest extends GridPanelBaseTest
// Column names.
private static final String FILTER_NAME_COL = "Name";
private static final String FILTER_EXPDATE_COL = "Expiration Date";
private static final String FILTER_STRING_COL = TestDataGenerator.randomFieldName("Str", 0, 5);
private static final String FILTER_INT_COL = TestDataGenerator.randomFieldName("Int", 0, 5);
private static final String FILTER_EXTEND_CHAR_COL = TestDataGenerator.randomFieldName("\u0106\u00D8\u0139", 0, 5);
private static final String FILTER_BOOL_COL = TestDataGenerator.randomFieldName("Bool", 0, 5);
private static final String FILTER_DATE_COL = TestDataGenerator.randomFieldName("Date", 0, 5);
Comment thread
labkey-tchad marked this conversation as resolved.
private static final String FILTER_STRING_COL = "Str";
private static final String FILTER_INT_COL = "Int";
private static final String FILTER_EXTEND_CHAR_COL = "\u0106\u00D8\u0139";
private static final String FILTER_BOOL_COL = "Bool";
private static final String FILTER_DATE_COL = "Date";
private static final String FILTER_STORED_AMOUNT_COL = "Amount";

// Views and columns used in the views. The views are only applied to the small sample type (Small_SampleType).
Expand Down Expand Up @@ -117,7 +118,7 @@ public class GridPanelTest extends GridPanelBaseTest
@BeforeClass
public static void setupProject() throws IOException, CommandException
{
GridPanelTest init = (GridPanelTest) getCurrentTest();
GridPanelTest init = getCurrentTest();

init.doSetup();
}
Expand Down Expand Up @@ -196,7 +197,7 @@ private void createSmallSampleType() throws IOException, CommandException
cv.saveCustomView(VIEW_FEWER_COLUMNS);

log(String.format("Finally create a view named '%s' for '%s' that only has a filter.", VIEW_FILTERED_COLUMN, SMALL_SAMPLE_TYPE));
drtSamples.setFilter(FILTER_STRING_COL, "Contains One Of", String.format("%1$s\n%1$s%2$s", stringSetMembers.get(0), stringSetMembers.get(1)));
drtSamples.setFilter(FieldKey.encodePart(FILTER_STRING_COL), "Contains One Of", String.format("%1$s\n%1$s%2$s", stringSetMembers.get(0), stringSetMembers.get(1)));
cv = drtSamples.openCustomizeGrid();
cv.saveCustomView(VIEW_FILTERED_COLUMN);

Expand Down
86 changes: 51 additions & 35 deletions src/org/labkey/test/tests/list/ListLookupTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.labkey.test.tests.list;

import org.apache.commons.csv.CSVFormat;
import org.jetbrains.annotations.Nullable;
import org.junit.BeforeClass;
import org.junit.Test;
Expand All @@ -16,10 +17,12 @@
import org.labkey.test.util.DataRegionTable;
import org.labkey.test.util.EscapeUtil;
import org.labkey.test.util.TestDataGenerator;
import org.labkey.test.util.data.TestDataUtils;
import org.labkey.test.util.query.QueryApiHelper;

import java.io.IOException;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Map;

Expand Down Expand Up @@ -55,11 +58,12 @@ private void doSetup()
log("Create a list to use as a lookup table with some number-like names.");
_listHelper.createList(getProjectName(), lookToListName, lookToKeyFieldName,
new FieldDefinition(lookToFieldName, FieldDefinition.ColumnType.String));
String bulkData = lookToFieldName + "\n" +
"1E2\n" +
"102\n" +
"Lookup\n" +
".123";
String bulkData = tsvFromColumn(List.of(
lookToFieldName,
"1E2",
"102",
"Lookup",
".123"));
_listHelper.bulkImportData(bulkData);

DataRegionTable dataRegionTable = new DataRegionTable("query", getDriver());
Expand All @@ -85,11 +89,12 @@ private void doSetup()
@Test
public void testWithoutValidatorOrAlternateKeys() throws IOException, CommandException
{
goToProjectHome();
setLookupValidatorEnabled(false);
resetList();

log("Import data into the second list without alternate keys.");
String bulkData = lookFromLookupFieldName + "\n" + lookupKeyAsNameNumber;
String bulkData = tsvFromColumn(List.of(lookFromLookupFieldName, lookupKeyAsNameNumber));
_listHelper.clickImportData()
.setText(bulkData)
.submit();
Expand All @@ -102,7 +107,7 @@ public void testWithoutValidatorOrAlternateKeys() throws IOException, CommandExc
log("Clean out list before next import.");
resetList();
log("Import data into second list without alternate keys supplying invalid primary key");
bulkData = lookFromLookupFieldName + "\n1000";
bulkData = tsvFromColumn(List.of(lookFromLookupFieldName, "1000"));
_listHelper.clickImportData()
.setText(bulkData)
.submit();
Expand All @@ -113,7 +118,7 @@ public void testWithoutValidatorOrAlternateKeys() throws IOException, CommandExc
validateListValues(expectedData);

log("Check for error if not using alternate key and type does not match");
bulkData = lookFromLookupFieldName + "\nnoneSuch";
bulkData = tsvFromColumn(List.of(lookFromLookupFieldName, "noneSuch"));
ImportDataPage importDataPage = _listHelper.clickImportData();
String error = importDataPage
.setText(bulkData)
Expand All @@ -124,20 +129,22 @@ public void testWithoutValidatorOrAlternateKeys() throws IOException, CommandExc
@Test
public void testWithoutValidatorWithAlternateKeys() throws IOException, CommandException
{
goToProjectHome();
setLookupValidatorEnabled(false);
log("Clean out list before next import.");
resetList();
log("Import data into the second list using number-like lookup values expecting alternate keys but also accepting primary keys.");
String bulkData = lookFromLookupFieldName + "\n" +
"1E2\n" + // valid alternate key looking like a number
lookupKeyAsNameNumber + "\n" + // valid alternate key same value as a primary key
".123\n" + // valid alternate key looking like a float
"Lookup\n" + // valid alternate key that is a string
lookupKeyAsNameNumber + "\n" + // another copy
"102\n" + // valid number-like alternate key
lookToListValues.get(1).get(EscapeUtil.fieldKeyEncodePart(lookToKeyFieldName)) + "\n" + // primary key value not matching an alternate key
"1000" // primary key-type value that doesn't match
;
String bulkData = tsvFromColumn(List.of(
lookFromLookupFieldName,
"1E2", // valid alternate key looking like a number
lookupKeyAsNameNumber, // valid alternate key same value as a primary key
".123", // valid alternate key looking like a float
"Lookup", // valid alternate key that is a string
lookupKeyAsNameNumber, // another copy
"102", // valid number-like alternate key
lookToListValues.get(1).get(EscapeUtil.fieldKeyEncodePart(lookToKeyFieldName)), // primary key value not matching an alternate key
"1000" // primary key-type value that doesn't match
));
_listHelper.clickImportData()
.setText(bulkData)
.setImportLookupByAlternateKey(true)
Expand All @@ -156,7 +163,7 @@ public void testWithoutValidatorWithAlternateKeys() throws IOException, CommandE
validateListValues(expectedData);

log("Check for error if providing non-matching string value that is not a number");
bulkData = lookFromLookupFieldName + "\nNotAValue";
bulkData = tsvFromColumn(List.of(lookFromLookupFieldName, "NotAValue"));
ImportDataPage importDataPage = _listHelper.clickImportData();
String error = importDataPage
.setText(bulkData)
Expand All @@ -168,13 +175,14 @@ public void testWithoutValidatorWithAlternateKeys() throws IOException, CommandE
@Test
public void testWithLookupValidatorWithoutAlternateKeys() throws IOException, CommandException
{
goToProjectHome();
setLookupValidatorEnabled(true);
log("Clean out list before next import.");
resetList();

// without alternate keys
log("With lookup validation on, import data into the second list without alternate keys.");
String bulkData = lookFromLookupFieldName + "\n" + lookupKeyAsNameNumber;
String bulkData = tsvFromColumn(List.of(lookFromLookupFieldName, lookupKeyAsNameNumber));
_listHelper.clickImportData()
.setText(bulkData)
.submit();
Expand All @@ -187,34 +195,36 @@ public void testWithLookupValidatorWithoutAlternateKeys() throws IOException, Co
log("With lookup validation on, import data and provide an invalid primary key.");
ImportDataPage importDataPage = _listHelper.clickImportData();
String error = importDataPage
.setText(lookFromLookupFieldName + "\n1000")
.setText(tsvFromColumn(List.of(lookFromLookupFieldName, "1000")))
.submitExpectingError();
checker().withScreenshot().verifyEquals("Error message for invalid primary key value not as expected", "Value '1000' was not present in lookup target 'lists." + lookToListName + "' for field '" + lookFromLookupFieldName + "'", error);
checker().withScreenshot().verifyEquals("Error message for invalid primary key value not as expected", "Value '1000' was not present in lookup target 'lists." + lookToListName + "' for field '" + FieldDefinition.labelFromName(lookFromLookupFieldName) + "'", error);

log("With lookup validation on, import data and provide an invalid primary key of type string.");
error = importDataPage
.setText(lookFromLookupFieldName + "\nLook")
.setText(tsvFromColumn(List.of(lookFromLookupFieldName, "Look")))
.submitExpectingError();
checker().withScreenshot().verifyEquals("Error message for invalid primary key type not as expected", "Could not convert value 'Look' (String) for Integer field '" + lookFromLookupFieldName + "'", error);
}

@Test
public void testWithLookupValidatorAndAlternateKeys() throws IOException, CommandException
{
goToProjectHome();
setLookupValidatorEnabled(true);
log("Clean out list before next import.");
resetList();

log("With lookup validation on, import data into the second list using number-like lookup values expecting alternate keys but also accepting primary keys.");
String bulkData = lookFromLookupFieldName + "\n" +
"1E2\n" + // valid alternate key looking like a number
lookupKeyAsNameNumber + "\n" + // valid alternate key same value as a primary key
".123\n" + // valid alternate key looking like a float
"Lookup\n" + // valid alternate key that is a string
lookupKeyAsNameNumber + "\n" + // another copy
"102\n" + // valid number-like alternate key
lookToListValues.get(1).get(EscapeUtil.fieldKeyEncodePart(lookToKeyFieldName)) + "\n" // primary key value not matching an alternate key
;
String bulkData = tsvFromColumn(List.of(
lookFromLookupFieldName,
"1E2", // valid alternate key looking like a number
lookupKeyAsNameNumber, // valid alternate key same value as a primary key
".123", // valid alternate key looking like a float
"Lookup", // valid alternate key that is a string
lookupKeyAsNameNumber, // another copy
"102", // valid number-like alternate key
lookToListValues.get(1).get(EscapeUtil.fieldKeyEncodePart(lookToKeyFieldName)) // primary key value not matching an alternate key
));
_listHelper.clickImportData()
.setText(bulkData)
.setImportLookupByAlternateKey(true)
Expand All @@ -231,20 +241,20 @@ public void testWithLookupValidatorAndAlternateKeys() throws IOException, Comman
);
validateListValues(expectedData);

bulkData = lookFromLookupFieldName + "\nInvalid";
bulkData = tsvFromColumn(List.of(lookFromLookupFieldName, "Invalid"));
ImportDataPage importDataPage = _listHelper.clickImportData();
String error = importDataPage
.setText(bulkData)
.setImportLookupByAlternateKey(true)
.submitExpectingError();
checker().withScreenshot().verifyEquals("Error message for invalid string alternate key not as expected", "Value 'Invalid' not found for field " + lookFromLookupFieldName + " in the current context.", error);

bulkData = lookFromLookupFieldName + "\n1234";
bulkData = tsvFromColumn(List.of(lookFromLookupFieldName, "1234"));
error = importDataPage
.setText(bulkData)
.setImportLookupByAlternateKey(true)
.submitExpectingError();
checker().withScreenshot().verifyEquals("Error message for invalid number-like alternate key not as expected", "Value '1234' was not present in lookup target 'lists." + lookToListName + "' for field '" + lookFromLookupFieldName + "'", error);
checker().withScreenshot().verifyEquals("Error message for invalid number-like alternate key not as expected", "Value '1234' was not present in lookup target 'lists." + lookToListName + "' for field '" + FieldDefinition.labelFromName(lookFromLookupFieldName) + "'", error);

}

Expand Down Expand Up @@ -273,6 +283,12 @@ private void validateListValues(List<Map<String, String>> expectedValue)
expectedValue, actualValue);
}

private String tsvFromColumn(List<String> column)
{
List<List<String>> rows = column.stream().map(Collections::singletonList).toList();
return TestDataUtils.stringFromRows(rows, CSVFormat.TDF);
}

@Override
protected @Nullable String getProjectName()
{
Expand Down
3 changes: 2 additions & 1 deletion src/org/labkey/test/util/TestDataGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,8 @@ public static String randomFieldName(String part, int numStartChars, int numEndC
public static String randomFieldName(@NotNull String part, int numStartChars, int numEndChars, @Nullable String exclusion)
{
// use the characters that we know are encoded in fieldKeys plus characters that we know clients are using
String chars = ALL_ILLEGAL_QUERY_KEY_CHARACTERS + " %()=+-[]_|*`'\":;<>?!@#^" + NON_LATIN_STRING + WIDE_PLACEHOLDER ;
// Issue 53197: Field name with double byte character can cause client side exception in Firefox when trying to customize grid view.
String chars = ALL_ILLEGAL_QUERY_KEY_CHARACTERS + " %()=+-[]_|*`'\":;<>?!@#^" + NON_LATIN_STRING;// + WIDE_PLACEHOLDER ;
Comment thread
labkey-tchad marked this conversation as resolved.

String randomFieldName = randomName(part, numStartChars, numEndChars, chars, exclusion);
TestLogger.log("Generated random field name: " + randomFieldName);
Expand Down