Skip to content

TestDataGenerator randomFieldName and randomDomainName usage of ValidateDomainAndFieldNamesAction#2571

Merged
cnathe merged 24 commits intodevelopfrom
fb_testDataGen258
Jul 22, 2025
Merged

TestDataGenerator randomFieldName and randomDomainName usage of ValidateDomainAndFieldNamesAction#2571
cnathe merged 24 commits intodevelopfrom
fb_testDataGen258

Conversation

@cnathe
Copy link
Contributor

@cnathe cnathe commented Jul 18, 2025

Rationale

We've been slowly handling various cases on the test code side to account for randomly generated domain and field names that end up being invalid (because of invalid character sequences, length, etc.). This PR changes that approach so that we instead call a server API (ValidateDomainAndFieldNamesAction) to do those checks on the generated name and regenerate the name if it is invalid.

Related Pull Requests

Changes

  • new FieldInfo(TestDataGenerator.randomFieldName() -> FieldInfo.random()
  • TestDataGenerator randomString support for REPEAT_PLACEHOLDER to add a repeated character
  • TestDataGenerator randomString support for ALL_CHARS_PLACEHOLDER to add all of the charSet to the generated string
  • DataClassAPIHelper.dataClassTestFields update to use FieldInfo.random()
  • PropertyController.ValidateDomainAndFieldNameAction and usage in TestDataGenerator.randomFieldName and randomDomainName

cnathe added 19 commits July 10, 2025 11:45
…dd all of the charSet to the generated string
…alidate domain names based on the domain kind
@cnathe cnathe requested a review from labkey-danield July 18, 2025 19:19
Comment on lines +70 to +71
testDgen.addCustomRow(Map.of("Name", "class1"));
testDgen.addCustomRow(Map.of("Name", "class2"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this just cleaning up data that adds no value to the test?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, these other columns like "intColumn" are now randomly generated names so when I went to update them in the data rows I noticed that the values aren't checked anywhere so it seemed cleaner to just remove the related data row values.

Comment on lines -84 to -95
// first the header
List<String> rows = new ArrayList<>();
rows.add(String.join("\t", data.get(0).keySet()));
data.forEach(dataMap -> {
StringBuilder row = new StringBuilder();
data.get(0).keySet().forEach(key -> {
row.append(dataMap.get(key));
row.append("\t");
});
rows.add(row.substring(0, row.lastIndexOf("\t")));
});
return String.join("\n", rows);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hopefully there aren't too many more methods like this out there.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've likely found most of them at this point!

Comment on lines +77 to +78
public static final char WIDE_PLACEHOLDER = '\u03A0'; // 'Π' - Wide character can't be picked from the string with 'charAt'
public static final char REPEAT_PLACEHOLDER = '\u22EF'; // '⋯' - Used to indicate that the char will be repeated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I was tempted on one of the previous PRs that made WIDE_CHAR public to suggest making WIDE_PLACEHOLDER and NON_LATIN_STRING public as well.

I also wonder how we decide where to put public constants like these as well as the next few lines. We have some constant values in TestDataUtils. BaseWebDriverTest has several constants as well. I think there might be value to consider a future PR that consolidates all of these type of constants into one location.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed!

@cnathe cnathe merged commit 193732e into develop Jul 22, 2025
9 of 10 checks passed
@cnathe cnathe deleted the fb_testDataGen258 branch July 22, 2025 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants