Skip to content

fix: Flaky test FunctionMetadataTest.mod #26441

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
aditi-pandit merged 7 commits into from
Nov 20, 2025
Merged

fix: Flaky test FunctionMetadataTest.mod #26441

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2f4af55
Fix failure of FunctionMetadataTest.mod on MacOS
rkurniawati Oct 26, 2025
6ee41d7
Fix code formatting
rkurniawati Oct 27, 2025
ad45824
Consolidate sorting of constraint arrays in metadata and metadata ele…
rkurniawati Oct 27, 2025
f6c7646
Use folly::hasher in the sortMetadataList, change sortMetadataList pa…
rkurniawati Oct 28, 2025
8586eb4
Fix test failure due to an extraneous metadata
rkurniawati Oct 28, 2025
50a1ebb
Sort the function metadata by the function kind and parameter types
rkurniawati Nov 4, 2025
8c705a2
Use basic_json::dump to convert json::array_t to string, remove unnec…
rkurniawati Nov 6, 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
29 changes: 23 additions & 6 deletions presto-native-execution/presto_cpp/main/functions/tests/FunctionMetadataTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,24 +40,41 @@ class FunctionMetadataTest : public ::testing::Test {
functionMetadata_ = getFunctionsMetadata();
}

void sortMetadataList(json::array_t& list) {
for (auto& metadata : list) {
// Sort constraint arrays for deterministic test comparisons.
for (auto const& [key, val] : metadata.items()) {
Copy link
Contributor

@aditi-pandit aditi-pandit Nov 6, 2025

Choose a reason for hiding this comment

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

Can you explain the relevant of this check in a comment.

Copy link
Contributor Author

@rkurniawati rkurniawati Nov 6, 2025

Choose a reason for hiding this comment

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

Done

if (key.ends_with("Constraints") && metadata[key].is_array()) {
std::sort(
metadata[key].begin(),
metadata[key].end(),
[](const json& a, const json& b) { return a.dump() < b.dump(); });
}
}
}
std::sort(list.begin(), list.end(), [](const json& a, const json& b) {
return folly::hasher<std::string>()(
a["functionKind"].dump() + a["paramTypes"].dump()) <
folly::hasher<std::string>()(
b["functionKind"].dump() + b["paramTypes"].dump());
});
}

void testFunction(
const std::string& name,
const std::string& expectedFile,
size_t expectedSize) {
json metadataList = functionMetadata_.at(name);
json::array_t metadataList = functionMetadata_.at(name);
EXPECT_EQ(metadataList.size(), expectedSize);
std::string expectedStr = slurp(
test::utils::getDataPath(
"/github/presto-trunk/presto-native-execution/presto_cpp/main/functions/tests/data/",
expectedFile));
auto expected = json::parse(expectedStr);
auto comparator = [](const json& a, const json& b) {
return (a["outputType"] < b["outputType"]);
};

json::array_t expectedList = expected[name];
std::sort(expectedList.begin(), expectedList.end(), comparator);
std::sort(metadataList.begin(), metadataList.end(), comparator);
sortMetadataList(expectedList);
sortMetadataList(metadataList);
for (auto i = 0; i < expectedSize; i++) {
EXPECT_EQ(expectedList[i], metadataList[i]) << "Position: " << i;
}
Expand Down
21 changes: 0 additions & 21 deletions presto-native-execution/presto_cpp/main/functions/tests/data/Variance.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,27 +104,6 @@
"schema": "default",
"typeVariableConstraints":[],
"variableArity":false
},
{
"aggregateMetadata": {
"intermediateType": "row(bigint,double,double)",
"isOrderSensitive": true
},
"docString": "presto.default.variance",
"functionKind": "WINDOW",
"longVariableConstraints":[],
"outputType": "double",
"paramTypes": [
"smallint"
],
"routineCharacteristics": {
"determinism": "DETERMINISTIC",
"language": "CPP",
"nullCallClause": "CALLED_ON_NULL_INPUT"
},
"schema": "default",
"typeVariableConstraints":[],
"variableArity":false
}
]
}
Loading