-
-
Notifications
You must be signed in to change notification settings - Fork 376
Issue 2808 wrap proname test functions #2816
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.
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
Issue 2808 wrap proname test functions #2816
Conversation
WalkthroughThis set of changes refactors SQL test scripts used throughout the pgtap suite. In nearly every file, calls that dynamically retrieved function argument names via subqueries (using functions such as Changes
Assessment against linked issues
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (74)
pgtap/allpairs/floydWarshall/types_check.pg
(1 hunks)pgtap/allpairs/johnson/types_check.pg
(1 hunks)pgtap/bdDijkstra/bdDijkstraCost/types_check.pg
(2 hunks)pgtap/bdDijkstra/bdDijkstraCostMatrix/types_check.pg
(1 hunks)pgtap/bellman_ford/bellman_ford/types_check.pg
(2 hunks)pgtap/bellman_ford/edwardMoore/types_check.pg
(3 hunks)pgtap/chinese/chinesePostman/types_check.pg
(1 hunks)pgtap/chinese/chinesePostmanCost/types_check.pg
(1 hunks)pgtap/circuits/hawickCircuits/types_check.pg
(1 hunks)pgtap/coloring/bipartite/types_check.pg
(1 hunks)pgtap/coloring/edgeColoring/types_check.pg
(1 hunks)pgtap/coloring/sequentialVertexColoring/types_check.pg
(1 hunks)pgtap/components/articulationPoints/types_check.pg
(1 hunks)pgtap/components/biconnectedComponents/types_check.pg
(1 hunks)pgtap/components/bridges/types_check.pg
(1 hunks)pgtap/components/connectedComponenes/types_check.pg
(1 hunks)pgtap/components/makeConnected/types_check.pg
(1 hunks)pgtap/components/strongComponenets/types_check.pg
(1 hunks)pgtap/contraction/contraction/types_check.pg
(1 hunks)pgtap/contraction/contractionDeadEnd/types_check.pg
(1 hunks)pgtap/contraction/contractionLinear/types_check.pg
(1 hunks)pgtap/dijkstra/dijkstraCost/types_check.pg
(2 hunks)pgtap/dijkstra/dijkstraCostMatrix/types_check.pg
(1 hunks)pgtap/dijkstra/dijkstraNear/types_check.pg
(1 hunks)pgtap/dijkstra/dijkstraNearCost/types_check.pg
(1 hunks)pgtap/dijkstra/driving_distance/types_check.pg
(2 hunks)pgtap/dominator/lengauerTarjanDominatorTree/types_check.pg
(1 hunks)pgtap/ksp/ksp/types_check.pg
(2 hunks)pgtap/ksp/turnRestrictedPath/types_check.pg
(1 hunks)pgtap/ksp/withPointsKSP/types_check.pg
(2 hunks)pgtap/lineGraph/lineGraph/types_check.pg
(1 hunks)pgtap/lineGraph/lineGraphFull/types_check.pg
(1 hunks)pgtap/max_flow/edgedisjointpaths/types_check.pg
(2 hunks)pgtap/max_flow/maxCardinalityMatch/types_check.pg
(2 hunks)pgtap/max_flow/maxFlow/types_check.pg
(1 hunks)pgtap/max_flow/maxFlowMinCost/types_check.pg
(2 hunks)pgtap/max_flow/maxFlowMinCost_Cost/types_check.pg
(1 hunks)pgtap/metrics/betweennessCentrality/types_check.pg
(1 hunks)pgtap/metrics/degree/types_check.pg
(2 hunks)pgtap/mincut/stoerWagner/types_check.pg
(1 hunks)pgtap/ordering/cuthillMckeeOrdering/types_check.pg
(1 hunks)pgtap/ordering/topologicalSort/types_check.pg
(1 hunks)pgtap/others/alpha_shape/types_check.pg
(1 hunks)pgtap/others/dagShortestPath/types_check.pg
(2 hunks)pgtap/others/transitiveClosure/types_check.pg
(1 hunks)pgtap/pickDeliver/pickDeliver/types_check.pg
(1 hunks)pgtap/pickDeliver/pickDeliverEuclidean/types_check.pg
(1 hunks)pgtap/pickDeliver/vrp_basic/types_check.pg
(1 hunks)pgtap/planar/isPlanar/types_check.pg
(1 hunks)pgtap/topology/analyzeGraph/types_check.pg
(1 hunks)pgtap/topology/analyzeOneWay/types_check.pg
(1 hunks)pgtap/topology/createTopology/types_check.pg
(1 hunks)pgtap/topology/createVerticesTable/types_check.pg
(1 hunks)pgtap/topology/extractVertices/types_check.pg
(1 hunks)pgtap/topology/nodeNetwork/types_check.pg
(1 hunks)pgtap/traversal/binaryBreadthFirstSearch/types_check.pg
(2 hunks)pgtap/traversal/breadthFirstSearch/types_check.pg
(1 hunks)pgtap/traversal/depthFirstSearch/types_check.pg
(1 hunks)pgtap/trsp/trsp/types_check.pg
(1 hunks)pgtap/trsp/trspViaEdges/types_check.pg
(1 hunks)pgtap/trsp/trspViaVertices/types_check.pg
(1 hunks)pgtap/utilities/findCloseEdges/types_check.pg
(2 hunks)pgtap/version/full_version/types_check.pg
(1 hunks)pgtap/withPoints/withPoints/types_check.pg
(2 hunks)pgtap/withPoints/withPointsCost/types_check.pg
(2 hunks)pgtap/withPoints/withPointsCostMatrix/types_check.pg
(1 hunks)pgtap/withPoints/withPointsDD/types_check.pg
(2 hunks)tools/testers/astar_pgtap_tests.sql
(6 hunks)tools/testers/dijkstra_pgtap_tests.sql
(3 hunks)tools/testers/flow_pgtap_tests.sql
(2 hunks)tools/testers/general_pgtap_tests.sql
(1 hunks)tools/testers/spanningtree.sql
(5 hunks)tools/testers/tsp_pgtap_tests.sql
(2 hunks)tools/testers/types_check.sql
(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (16)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Ubuntu Boost (79)
- GitHub Check: Ubuntu Boost (84)
- GitHub Check: Ubuntu Boost (86)
- GitHub Check: Ubuntu Boost (80)
- GitHub Check: macos (macos-latest)
- GitHub Check: macos (macos-14)
- GitHub Check: Ubuntu Boost (77)
- GitHub Check: Ubuntu Boost (75)
- GitHub Check: Check queries
- GitHub Check: Ubuntu Boost (76)
- GitHub Check: Ubuntu Boost (83)
- GitHub Check: Ubuntu Boost (78)
- GitHub Check: Ubuntu Boost (56)
- GitHub Check: Ubuntu Boost (68)
- GitHub Check: Ubuntu clang
🔇 Additional comments (104)
pgtap/chinese/chinesePostman/types_check.pg (1)
29-31
: Improved function argument testingThe change from a dynamic subquery to using the dedicated
function_args_eq
function improves test clarity and maintainability. This approach makes it more explicit what arguments are expected from thepgr_chinesepostman
function.pgtap/pickDeliver/vrp_basic/types_check.pg (1)
27-29
: Improved function argument testingThe change from a dynamic subquery to using the dedicated
function_args_eq
function improves test clarity and maintainability. This approach makes it more explicit what arguments are expected from thepgr_vrponedepot
function.pgtap/version/full_version/types_check.pg (1)
31-33
: Improved function argument testingThe change from a dynamic subquery to using the dedicated
function_args_eq
function improves test clarity and maintainability. This approach makes it more explicit what arguments are expected from thepgr_full_version
function.pgtap/traversal/depthFirstSearch/types_check.pg (1)
44-46
: Improved function argument testingThe change from using
bag_has
with a subquery to using the dedicatedfunction_args_eq
function improves test clarity and maintainability. This approach makes it more explicit what arguments are expected from thepgr_depthfirstsearch
function.pgtap/components/connectedComponenes/types_check.pg (1)
28-30
: Improved test clarity with function_args_eqThe replacement of
set_eq
withfunction_args_eq
improves test clarity by explicitly defining the expected function arguments rather than dynamically retrieving them. This makes the test more readable and easier to maintain.pgtap/coloring/sequentialVertexColoring/types_check.pg (1)
40-42
: Enhanced argument validation with function_args_eqThe replacement of
bag_has
withfunction_args_eq
strengthens the test by explicitly defining the expected function arguments and performing an exact equality check. This improves test clarity and makes the expected structure ofpgr_sequentialvertexcoloring
arguments more visible.pgtap/coloring/bipartite/types_check.pg (1)
40-42
: Consistent implementation of function_args_eqThe replacement of
bag_has
withfunction_args_eq
follows the same pattern applied in other test files, ensuring consistency across the test suite while making the expected arguments forpgr_bipartite
explicitly clear.pgtap/topology/createTopology/types_check.pg (1)
28-30
: Consistent argument validation patternThe change from
set_eq
tofunction_args_eq
for validating function arguments follows the same pattern applied throughout the codebase. The test correctly maintains the separateset_eq
check for the function's return type while improving the clarity of argument validation.pgtap/withPoints/withPointsCostMatrix/types_check.pg (1)
28-31
: LGTM: Clean migration to usingfunction_args_eq
The change replaces a previous
set_eq
call with the more directfunction_args_eq
function, which provides a cleaner approach to validate function signatures. This matches the pattern described in the PR objectives for wrapping the tests.pgtap/circuits/hawickCircuits/types_check.pg (1)
43-46
: LGTM: Properly replaced query with direct function callThe replacement of
set_eq
withfunction_args_eq
simplifies the test by directly providing the expected arguments instead of querying the system catalog. This is consistent with the PR's objectives and makes the test more maintainable.pgtap/lineGraph/lineGraph/types_check.pg (1)
28-30
: LGTM: Improved argument validation approachThe change properly replaces the dynamic SQL-based
set_eq
method with the more directfunction_args_eq
approach. Note that this file uses a different syntax style (usingSELECT
instead of direct values) compared to the other files, but the functionality is equivalent.pgtap/max_flow/maxFlowMinCost/types_check.pg (2)
54-62
: LGTM: Successfully replaced set_eq with function_args_eq in 3.2.0+ conditionThe function_args_eq implementation properly handles argument validation for both versions (3.2.0+) of the function, including the additional signature added in 3.2.0.
75-82
: LGTM: Successfully replaced set_eq with function_args_eq in pre-3.2.0 conditionThe function_args_eq implementation correctly handles the pre-3.2.0 version with only the four original function signatures. The conditional structure is preserved, maintaining backward compatibility.
pgtap/components/bridges/types_check.pg (1)
29-31
: Use offunction_args_eq
improves code maintainabilityThe change from a subquery-based approach to using the dedicated
function_args_eq
function improves code clarity and maintenance. This refactoring provides a more direct and explicit way to validate function arguments.pgtap/ordering/topologicalSort/types_check.pg (1)
34-36
: Appropriate use offunction_args_eq
for argument validationThe change from what appears to be a
bag_has
with subquery tofunction_args_eq
improves consistency with the rest of the codebase and makes the expected function arguments more explicit and easier to validate.pgtap/trsp/trsp/types_check.pg (1)
33-37
: Usingfunction_args_has
for multiple function signaturesGood choice of using
function_args_has
instead offunction_args_eq
here, as this test is checking for the presence of specific arguments across multiple function overloads rather than exact argument equality.pgtap/ksp/ksp/types_check.pg (2)
48-50
: Good refactoring to use the new helper functionReplacing the old
set_eq
query with the newfunction_args_eq
helper function improves code clarity by removing the subquery complexity while maintaining the same validation logic.
66-67
: Appropriate use of the helper function in the ELSE branchConsistently applying the same pattern here in the ELSE branch maintains code uniformity and improves maintainability.
pgtap/components/articulationPoints/types_check.pg (1)
29-31
: Good refactoring to use the standardized helper functionThe change to
function_args_eq
simplifies the code while maintaining the same validation logic, consistent with the project-wide standardization effort.pgtap/lineGraph/lineGraphFull/types_check.pg (1)
28-30
: Good refactoring to use the standardized helper functionThe change to
function_args_eq
follows the same pattern as other files in this PR, improving consistency across the test suite while maintaining the same validation approach.pgtap/pickDeliver/pickDeliverEuclidean/types_check.pg (1)
30-34
: Good refactoring to standardize validation of complex function argumentsThe change to
function_args_eq
is particularly beneficial here where the function has many arguments. The standardized approach removes what would have been a complex subquery while making the tests consistent with the rest of the suite.pgtap/dijkstra/dijkstraCost/types_check.pg (2)
54-62
: Clear refactoring to standardized test approachThe change from what was likely a
set_eq
with a subquery to the newfunction_args_eq
function improves readability and maintainability by directly specifying the expected function arguments rather than dynamically querying them.
74-81
: Consistent application of the refactoring approachThe refactoring pattern is consistently applied here for the non-3.1.0 version case, using the same
function_args_eq
approach. This maintains consistency across different conditional branches of the code.pgtap/ordering/cuthillMckeeOrdering/types_check.pg (1)
39-42
: Simplified test query using new helper functionThe refactoring replaces what was likely a
bag_eq
function call with a subquery with the newfunction_args_eq
helper. This approach is more direct and easier to maintain as it eliminates the need for subqueries to fetch function arguments from pg_proc.pgtap/ksp/turnRestrictedPath/types_check.pg (1)
29-31
: Standardized approach to function argument testingThe change replaces a dynamic query with the new
function_args_eq
helper function, following the consistent pattern applied throughout the codebase. The expected arguments include all the necessary parameters: directed, heap_paths, stop_on_first, strict, seq, path_id, path_seq, node, edge, cost, and agg_cost.pgtap/topology/analyzeOneWay/types_check.pg (2)
28-30
: Consistent refactoring to use standardized helper functionThe change follows the same pattern as other files by replacing a dynamic query with the
function_args_eq
helper function. This makes the code more maintainable and easier to read.
36-39
: Preserved set_eq for return type checkingNote that while argument checking has been refactored to use
function_args_eq
, thisset_eq
call for checking the return type is correctly preserved. This suggests a deliberate decision to only replace argument checking patterns while keeping other checks intact.pgtap/traversal/breadthFirstSearch/types_check.pg (1)
33-38
:✅ Verification successful
Implementation of
function_args_eq
simplifies argument validationThe change replaces a likely SQL query-based approach with a more straightforward function call to
function_args_eq
, which directly compares the function's argument names against expected values. This improves clarity and maintainability.I noticed the expected arguments array has the same value twice. Please verify if this duplication is intentional or if it should contain different values for different function signatures.
🏁 Script executed:
#!/bin/bash # Check if there are two overloads of pgr_breadthfirstsearch with different args pg_proc_count=$(rg -o "pgr_breadthfirstsearch.*ARRAY.*boolean" pgtap/traversal/breadthFirstSearch/types_check.pg | wc -l) echo "Number of pgr_breadthfirstsearch function signatures: $pg_proc_count"Length of output: 247
Clarification on Duplicate Argument Arrays in
pgr_breadthfirstsearch
Signature TestingAfter verification, it appears that the file registers four overloads of the
pgr_breadthfirstsearch
function. The test provides two expected argument arrays that are identical, which suggests that the overloads may be grouped by two distinct signatures that happen to use the same argument list. Unless there’s a documented difference elsewhere or a need to differentiate between subtle type variations, this duplication is intentional to cover those grouped overloads.No changes to the expected arrays are necessary based on the current evidence.
pgtap/dijkstra/dijkstraCostMatrix/types_check.pg (1)
28-31
: Function validation refactoring improves code clarityThe implementation now uses
function_args_eq
to directly verify function arguments against expected values, replacing what was likely a more complex query-based validation. This change aligns with the PR objective of wrapping function tests and provides a more standardized approach to argument validation.pgtap/traversal/binaryBreadthFirstSearch/types_check.pg (2)
57-67
: Enhanced version-specific argument validation with cleaner syntaxThe refactoring replaces a likely query-based validation with a direct function call to
function_args_eq
, providing better readability and maintainability when checking function signatures for version 3.2.0 and above. The implementation correctly handles the different signatures available in newer versions.
80-88
: Consistent approach to argument validation for older versionsThe implementation for older versions (<3.2.0) follows the same pattern as the newer version, except it correctly omits the fifth function signature that's only available in version 3.2.0 and above. This consistent approach makes the code more maintainable across version-specific branches.
pgtap/components/biconnectedComponents/types_check.pg (1)
29-31
: Simplified argument validation with SQL-friendly syntaxThe change replaces a likely more complex validation with a cleaner
function_args_eq
implementation. The approach uses a SQL SELECT to create the expected arguments array, which differs slightly from the VALUES approach in other files but achieves the same goal. This variation in syntax is appropriate for a single set of arguments.pgtap/bdDijkstra/bdDijkstraCostMatrix/types_check.pg (1)
28-31
: Simplified argument verification approach with direct function callThe change replaces a dynamic subquery-based approach with a simpler direct function call (
function_args_eq
), which improves readability and maintainability. Using a hardcoded array of expected arguments follows a more consistent pattern across all test files.pgtap/bdDijkstra/bdDijkstraCost/types_check.pg (2)
54-62
: Improved argument verification using function_args_eqReplacing the
set_eq
withfunction_args_eq
streamlines the validation of function arguments and improves consistency across test files. The hardcoded array approach is more direct and easier to maintain than the previous dynamic subquery method.
75-82
: Consistent approach for legacy version compatibilityGood consistency in applying the same pattern for the non-3.2.0 version case. This approach maintains uniform testing methodology across different PostgreSQL versions.
pgtap/components/strongComponenets/types_check.pg (1)
29-31
:❓ Verification inconclusive
Enhanced argument validation with direct function call
The replacement of
set_eq
withfunction_args_eq
simplifies the test logic by eliminating the dynamic subquery and using a direct comparison with expected argument names.
🏁 Script executed:
#!/bin/bash # Check for filename typo in the directory path fd "strongComponenets" --type d # The correct spelling should be "strongComponents" - this could indicate a typo in the directory structureLength of output: 67
Action Required: Confirm Directory Naming & Validate Test Update
- The updated code in
pgtap/components/strongComponenets/types_check.pg
now uses a direct call tofunction_args_eq
for argument validation, which simplifies the testing logic.- The shell script confirms that the directory is named
strongComponenets
. If this spelling is unintentional, please review whether the directory should be renamed tostrongComponents
.pgtap/components/makeConnected/types_check.pg (1)
39-41
: Standardized argument verification using function_args_eqThis change follows the consistent pattern of replacing dynamic subqueries with the direct
function_args_eq
function call, improving readability and maintainability of the test code.pgtap/dijkstra/dijkstraNearCost/types_check.pg (1)
51-58
: Improved test readability withfunction_args_eq
The change from using a dynamic SQL query with
set_eq
to the more declarativefunction_args_eq
function makes the test clearer and more maintainable. This approach directly specifies what's being tested rather than relying on complex subqueries to retrieve function arguments from the PostgreSQL catalog.This refactoring aligns with the PR objective of wrapping test functions for better organization without changing the actual test behavior or expected results.
pgtap/topology/analyzeGraph/types_check.pg (1)
28-30
: Simplified argument testing withfunction_args_eq
Good refactoring to use the dedicated
function_args_eq
function instead of a complex SQL query. This makes the test more readable and consistent with the other changes in this PR. The expected arguments forpgr_analyzegraph
remain the same, but the way they're verified is now more direct and declarative.pgtap/dijkstra/driving_distance/types_check.pg (2)
39-43
: Improved version-specific argument testingGood implementation of
function_args_eq
for the PostgreSQL 3.6.0+ case. The refactoring maintains the version-specific testing logic while making the test more readable and consistent with other files in this PR.
54-58
: Consistent refactoring for older PostgreSQL versionsThe refactoring to
function_args_eq
is consistently applied to the older version case as well, maintaining the same expected arguments but with a cleaner implementation. This ensures consistent testing approaches across different PostgreSQL versions while making the code more maintainable.pgtap/others/transitiveClosure/types_check.pg (1)
34-36
: Replacedbag_has
with more directfunction_args_eq
Good refactoring from
bag_has
tofunction_args_eq
, which provides a more consistent approach to testing function arguments across the codebase. The expected arguments forpgr_transitiveclosure
remain unchanged, but the verification method is now more direct and follows the same pattern used in other test files.pgtap/topology/createVerticesTable/types_check.pg (1)
28-30
: Improved function argument validation approachThe change replaces a SQL-based comparison with the new
function_args_eq
function, which makes the code more readable and maintainable. This is a positive refactoring that aligns with similar changes across the codebase.pgtap/planar/isPlanar/types_check.pg (1)
40-42
: Good replacement of bag_has with function_args_eqReplacing the
bag_has
approach with the newfunction_args_eq
function makes the code more consistent with other tests while maintaining the same functionality. This type of standardization will make the test suite easier to maintain.pgtap/metrics/betweennessCentrality/types_check.pg (1)
43-46
: Well-structured function argument validationThe new implementation using
function_args_eq
with a VALUES expression is cleaner and more readable than the previous approach usingset_eq
. The argument names specified correctly match the expected structure for the betweenness centrality function.pgtap/withPoints/withPointsDD/types_check.pg (2)
49-57
: Good refactoring of version-specific function argument testsThe implementation nicely handles the more complex case with multiple function signatures in the VALUES expression. This approach is more maintainable than the previous SQL-based comparison and keeps all expected argument names in one place.
67-72
: Consistent approach for fallback version testsThe same refactoring approach is properly applied to the fallback case for older PostgreSQL versions, maintaining consistency throughout the test suite while preserving the different expected arguments for each version.
pgtap/trsp/trspViaVertices/types_check.pg (1)
31-34
: Good refactoring using function_args_eqThe change from using
set_eq
with SQL queries to the more directfunction_args_eq
improves code maintainability and readability. This refactoring pattern is consistent with the PR objective of wrapping test functions.pgtap/bellman_ford/edwardMoore/types_check.pg (2)
56-65
: Clean refactoring to function_args_eqGood replacement of the
set_eq
with the more directfunction_args_eq
helper function. This improves code readability by abstracting away the SQL query details while maintaining the same functionality.
79-87
: Consistent refactoring in ELSE branchThe refactoring to
function_args_eq
is consistently applied in the ELSE branch as well, maintaining the same pattern throughout the conditional logic. This consistency is good for maintainability.tools/testers/flow_pgtap_tests.sql (2)
191-199
: Good migration to function_args_eq in flow testsThe refactoring from
set_eq
tofunction_args_eq
is well implemented. The VALUES syntax and expected argument arrays are properly maintained in the transition.
212-219
: Consistent implementation in ELSE branchThe refactoring pattern is consistently applied in the ELSE branch, maintaining the same approach for version compatibility handling. This ensures the tests work correctly across different PostgreSQL versions.
pgtap/metrics/degree/types_check.pg (2)
45-49
: Clean conversion to function_args_eqThe migration from
set_eq
tofunction_args_eq
is well implemented. The expected argument arrays format is maintained correctly during the refactoring.
60-61
: Concise implementation in ELSE branchThe refactoring in the ELSE branch maintains the same pattern but with the correct simplified argument list for the older version case. The syntax is properly adjusted to work with the new helper function.
pgtap/mincut/stoerWagner/types_check.pg (1)
30-33
: LGTM: Good refactoring to use function_args_eqThe change from using a direct SQL query with
set_eq
to the more declarativefunction_args_eq
function is a positive improvement. This approach reduces complexity while maintaining the same validation functionality, making the code more maintainable and consistent with other test files.pgtap/topology/extractVertices/types_check.pg (1)
28-30
: LGTM: Clean refactoring to use function_args_eqThe replacement of the SQL query approach with the dedicated
function_args_eq
function makes the code cleaner and more maintainable. This change aligns with the PR objective of standardizing function testing patterns across pgtap tests.pgtap/dominator/lengauerTarjanDominatorTree/types_check.pg (1)
41-43
: LGTM: Improved argument validation approachReplacing the
bag_has
query withfunction_args_eq
is a good refactoring that simplifies the code while maintaining the same validation functionality. This change is consistent with the standardization pattern applied throughout the pgtap test suite.pgtap/withPoints/withPointsCost/types_check.pg (2)
54-62
: LGTM: Cleaner function argument validationThe refactoring to use
function_args_eq
instead of direct SQL queries improves code readability and maintainability. This approach provides a more standardized way to verify function arguments across the test suite.
75-82
: LGTM: Consistent refactoring in the else branchThe same refactoring pattern is correctly applied in the else branch of the conditional, ensuring consistent behavior regardless of the version being tested against. This demonstrates thorough attention to detail in the refactoring process.
tools/testers/dijkstra_pgtap_tests.sql (3)
154-158
: Refactoring to use the new helper function improves code readabilityThe change from directly querying the
pg_proc
table to using the newly createdfunction_args_eq
helper function enhances maintainability while preserving the same functionality.
176-183
: Good refactoring that follows DRY principlesReplacing the direct SQL query with the
function_args_eq
helper function reduces code duplication across test files and centralizes the logic for argument name verification.
195-201
: Consistent refactoring improves code maintainabilityThis change maintains the same functionality while making the code more maintainable by using the centralized helper function instead of repeating SQL queries.
tools/testers/general_pgtap_tests.sql (2)
85-91
: Well-designed helper function for argument name equality checkingThis new function encapsulates the logic for checking whether a function's argument names match expected values, making the codebase more maintainable and following the DRY principle.
93-99
: Good companion function for subset checking of argument namesThis helper function complements
function_args_eq
by providing a way to check if a function's argument names include a specified subset, rather than requiring an exact match.pgtap/bellman_ford/bellman_ford/types_check.pg (2)
57-58
: Consistent use of the new helper functionThe refactoring to use
function_args_eq
aligns with changes throughout the codebase, improving maintainability while preserving the same validation logic.
80-81
: Consistent refactoring pattern continues in else branchThis change follows the same pattern as the previous section, ensuring the helper function is used consistently throughout the conditional branches.
pgtap/chinese/chinesePostmanCost/types_check.pg (1)
29-31
: Refactoring maintains the NULL argument name checkGood use of the helper function while preserving the check for NULL argument names, indicating this function doesn't have named arguments.
pgtap/allpairs/floydWarshall/types_check.pg (1)
28-31
: Good refactoring - improved code maintainability.The change replaces a direct query to the PostgreSQL system catalog (
pg_proc
) with a dedicated helper functionfunction_args_eq
. This approach is cleaner and more maintainable, decoupling the test from the internal details of how PostgreSQL stores function metadata.Note that the argument format has changed from quoted JSON elements
{"","directed","start_vid","end_vid","agg_cost"}
to a format with unquoted identifiers{"",directed,start_vid,end_vid,agg_cost}
. This syntactic change maintains the same functionality.pgtap/dijkstra/dijkstraNear/types_check.pg (1)
50-57
: Good abstraction with function_args_eq.The change simplifies the code by replacing a direct system catalog query with the
function_args_eq
helper function. This is part of a consistent pattern across the codebase that improves maintainability by centralizing how function arguments are verified.The function signatures and expected argument lists appear to be unchanged, maintaining test coverage while improving code clarity.
pgtap/topology/nodeNetwork/types_check.pg (1)
28-30
: Good consistency improvement with helper function.This change follows the same pattern as in other files, replacing a direct query to
pg_proc
with thefunction_args_eq
helper function. The expected argument list is preserved ({"","","id","the_geom","table_ending","rows_where","outall"}
), maintaining the same test functionality while improving code consistency and maintainability.pgtap/withPoints/withPoints/types_check.pg (2)
53-61
: Good refactoring using helper function in conditional block.The change replaces a direct system catalog query with the
function_args_eq
helper function in the conditional block for version 3.2.0 or higher. This follows the consistent pattern across the codebase and improves maintainability by abstracting the details of how function arguments are retrieved and compared.
74-81
: Good refactoring in else block ensures consistent approach.Similar to the change above, this replaces the direct system catalog query with the
function_args_eq
helper function in the ELSE block for versions below 3.2.0. Ensuring the same approach is used in both conditional paths maintains consistency and makes the code easier to understand and maintain.pgtap/pickDeliver/pickDeliver/types_check.pg (1)
30-33
: Good refactoring to a more explicit testing approachThe change from using a dynamic subquery with
set_eq
to using the more specificfunction_args_eq
function is a good improvement. This approach makes the test more self-contained and explicit about the expected function arguments.The static array definition clearly shows what arguments are expected for the
pgr_pickdeliver
function, which improves readability and maintainability.pgtap/max_flow/maxFlow/types_check.pg (1)
47-48
: Consistent refactoring pattern for function argument checkingThis modification follows the same pattern as other files, replacing a dynamic query with the more direct
function_args_eq
function. The test is checking that thepgr_maxflow
function has no named arguments (NULL::TEXT[]).This change aligns with the overall goal of standardizing argument validation across the codebase.
pgtap/coloring/edgeColoring/types_check.pg (1)
40-42
: Improved approach for argument validationThe change from using
bag_has
with a subquery tofunction_args_eq
is a good improvement. The new approach directly specifies the expected arguments{"","edge_id","color_id"}
for thepgr_edgecoloring
function in a clearer, more maintainable way.This refactoring makes the test more explicit and easier to understand.
pgtap/contraction/contractionLinear/types_check.pg (1)
37-41
: Good standardization using VALUES syntaxThe refactoring to use
function_args_eq
with a VALUES clause is a good approach. This implementation style clearly defines the expected arguments for thepgr_contractionlinear
function while maintaining consistency with the overall refactoring pattern.Note that this implementation uses the VALUES syntax rather than a string with SELECT, which is a valid alternative approach that achieves the same result.
pgtap/max_flow/maxCardinalityMatch/types_check.pg (1)
35-41
: Looks good - consistent argument formatThe refactored code correctly checks the expected arguments for the
pgr_maxcardinalitymatch
function when the version is ≥ 3.4.0, and it properly uses theVALUES
clause to check multiple function signatures.pgtap/trsp/trspViaEdges/types_check.pg (1)
32-35
: Looks good - consistent argument formatThe refactored code correctly checks the expected arguments for the
pgr_trspviaedges
function, using theVALUES
clause with properly formatted expected arguments.tools/testers/astar_pgtap_tests.sql (6)
33-41
: Consistent refactoring of argument verification approach.The replacement of
set_eq
withfunction_args_eq
standardizes how function arguments are verified throughout the codebase. This change makes the testing approach more modular and maintainable.
52-60
: Function verification pattern consistently applied.The modification maintains the same argument verification approach as in the previous section, applying the new
function_args_eq
helper function. This creates a uniform testing pattern across different version conditions.
70-77
: Consistent application of the new argument verification pattern.The replacement with
function_args_eq
completes the refactoring pattern for all code paths in this function. The change preserves the original functionality while improving code structure.
123-131
: Appropriate implementation of function_args_eq in astarcost_types_check.The refactoring pattern is correctly applied to the
astarcost_types_check
function using the same approach as in previous test functions.
141-148
: Consistent application of the verification pattern for older versions.The refactoring appropriately handles the else condition, maintaining consistent verification approaches across different version paths.
180-184
: Clean implementation of function_args_eq in astarcostmatrix_types_check.This simple verification case has been properly refactored to use the new helper function, maintaining consistency with other test functions.
pgtap/contraction/contraction/types_check.pg (2)
32-37
: Well-structured refactoring to use function_args_eq.The test has been properly updated to use the new helper function rather than a direct
set_eq
call. The change maintains the same verification logic while improving code organization.
45-49
: Consistent pattern applied to the else condition.The refactoring correctly applies the new verification approach in the else branch of the conditional, ensuring a uniform testing pattern throughout the function.
pgtap/max_flow/edgedisjointpaths/types_check.pg (2)
50-58
: Appropriate implementation of function_args_eq within collect_tap.The refactoring correctly replaces
set_eq
withfunction_args_eq
while maintaining the proper integration withcollect_tap
. This approach ensures that the test results are properly collected regardless of the verification method used.
71-78
: Consistent refactoring in the else branch of the conditional.The change maintains the same pattern as in the previous branch, ensuring a uniform verification approach across different version conditions.
tools/testers/spanningtree.sql (5)
9-11
: Clean implementation in spanntree_types_check function.The refactoring correctly replaces a direct SQL query parameter with the new helper function, making the test more readable and consistent with the rest of the codebase.
34-38
: Appropriate implementation in spanntree_bfs_types_check.The function's verification logic has been properly refactored to use the new helper function, maintaining the same testing behavior.
49-53
: Consistent pattern applied in the else branch.The refactoring correctly applies the new verification approach in the else condition, ensuring consistent testing across different version conditions.
86-90
: Well-structured refactoring in spanntree_dd_types_check.The test function has been properly updated to use the new helper function, maintaining the same verification approach as other test functions.
105-109
: Consistent application of the pattern in the final else condition.The change properly implements the new verification approach in the last conditional branch, completing the refactoring pattern across all test functions.
pgtap/ksp/withPointsKSP/types_check.pg (1)
44-50
: Good refactoring of argument validation!The replacement of what was likely a direct database query with
function_args_eq
simplifies the code and makes the intent clearer. This approach is more maintainable and follows a consistent pattern being applied across the codebase.tools/testers/tsp_pgtap_tests.sql (2)
182-182
: Clean implementation of function_args_eqNice simplification of the parameter validation using the new helper function.
219-226
: Good refactoring to standardize argument validationThe replacement of the previous validation method with
function_args_eq
creates a more consistent approach throughout the test suite. Line 226 now properly includes the "randomize" parameter in the same string literal as the other parameters.pgtap/contraction/contractionDeadEnd/types_check.pg (1)
37-41
: Improved argument validation approachThe change from what was likely a direct database query to using
function_args_eq
is a good refactoring that aligns with the broader standardization effort in the codebase. This makes the tests more maintainable and easier to understand.pgtap/others/dagShortestPath/types_check.pg (2)
56-64
: Good implementation of function_args_eq with additional test caseThe refactoring to use
function_args_eq
matches the pattern used in other files. I notice you've added an additional argument set in line 63 for the text-text signature, which provides more complete test coverage.
80-88
: Consistent implementation of function_args_eqThis change maintains consistency with the rest of the codebase by using the
function_args_eq
function instead of the previous validation approach.pgtap/utilities/findCloseEdges/types_check.pg (2)
47-54
: Refactoring to usefunction_args_eq
instead ofbag_eq
The change replaces
bag_eq
withfunction_args_eq
for checking function arguments, which aligns with the PR's objective of wrapping proname test functions. The expected arguments remain the same, suggesting this is part of a standardization effort across the codebase.
67-72
: Consistent refactoring in conditional branchThis change maintains consistency with the previous branch by also replacing
bag_eq
withfunction_args_eq
in the code path for PostgreSQL versions below 3.8.0. The arguments passed remain unchanged, preserving the test's original functionality.tools/testers/types_check.sql (3)
85-103
: Refactoring to use specialized function for argument checkingThe change from
bag_eq
tofunction_args_eq
is consistent with the PR's objective of wrapping proname test functions for better organization. The arguments and formatting remain unchanged, maintaining the same test behavior while improving code consistency.
129-134
: Continued standardization of function argument checkingThis continues the pattern of replacing
bag_eq
withfunction_args_eq
in the combinations section of the function. The consistency in approach demonstrates a thoughtful refactoring strategy.
238-245
: Consistent use offunction_args_eq
in types_check_via functionThe change from
bag_eq
tofunction_args_eq
is applied consistently across the codebase, including thetypes_check_via
function. This promotes uniform testing methodology throughout the project.
Fixes #2808
Changes proposed in this pull request:
@pgRouting/admins
Summary by CodeRabbit
These changes are purely internal improvements that strengthen the robustness of our system without affecting any end-user functionality.