[SPARK-31526][SQL][TESTS] Add a new test suite for ExpressionInfo #28308
Conversation
|
|
||
| // The groups below are almost the same with ones defined in `sql/functions.sql`. | ||
| // But, `collection_funcs` is split into fine-grained three groups: `array_funcs`, `map_funcs`, | ||
| // and `json_funcs`. |
There was a problem hiding this comment.
This is a minor follow-up for #28224, @HyukjinKwon
There was a problem hiding this comment.
Nice but what about we document this in ExpressionDescription, and here we say things like please refer ExpressionDescription?
There was a problem hiding this comment.
Yea, the idea looks nice. I'll update.
| assert(errMsg.contains("'group' is malformed in the expression [testName].")) | ||
| } | ||
|
|
||
| test("error handling in ExpressionInfo") { |
There was a problem hiding this comment.
To improve test coverage, a new test added here.
|
|
||
| class ExpressionInfoSuite extends SparkFunSuite with SharedSparkSession { | ||
|
|
||
| test("Replace _FUNC_ in ExpressionInfo") { |
| assert(info.getExtended.contains("> SELECT upper('SparkSql');")) | ||
| } | ||
|
|
||
| test("group info in ExpressionInfo") { |
| assert(errMsg3.contains("'deprecated' is malformed in the expression [testName].")) | ||
| } | ||
|
|
||
| test("using _FUNC_ instead of function names in examples") { |
| } | ||
| } | ||
|
|
||
| test("check outputs of expression examples") { |
| withClue(s"Function '${info.getName}', Expression class '$className'") { | ||
| val example = info.getExamples | ||
| checkExampleSyntax(example) | ||
| example.split(" > ").toList.foreach { |
There was a problem hiding this comment.
minor: I modified this line a bit as my IDE suggested.
| private String since; | ||
| private String deprecated; | ||
|
|
||
| // The groups below are almost the same with ones defined in `sql/functions.sql`. |
There was a problem hiding this comment.
maybe you meant functions.sql -> functions.scala?
| /** | ||
| * Valid group names are almost the same with one defined as `groupname` in | ||
| * `sql/functions.scala`. But, `collection_funcs` is split into fine-grained three groups: | ||
| * `array_funcs`, `map_funcs`, and `json_funcs`. See `ExpressionInfo` for the |
There was a problem hiding this comment.
Why all the csv _funcs are left behind?
There was a problem hiding this comment.
This is just a basic set for the 3.0 release. Yea, we can improve docs by adding more groups. Actually, we need to assign all the exprs to groups for removing the SQL Built-in Function doc:
#28224 (comment) @HyukjinKwon
|
Test build #121661 has finished for PR 28308 at commit
|
|
Test build #121662 has finished for PR 28308 at commit
|
|
Merged to master and branch-3.0. |
### What changes were proposed in this pull request? This PR intends to add a new test suite for `ExpressionInfo`. Major changes are as follows; - Added a new test suite named `ExpressionInfoSuite` - To improve test coverage, added a test for error handling in `ExpressionInfoSuite` - Moved the `ExpressionInfo`-related tests from `UDFSuite` to `ExpressionInfoSuite` - Moved the related tests from `SQLQuerySuite` to `ExpressionInfoSuite` - Added a comment in `ExpressionInfoSuite` (followup of #28224) ### Why are the changes needed? To improve test suites/coverage. ### Does this PR introduce any user-facing change? No. ### How was this patch tested? Added tests. Closes #28308 from maropu/SPARK-31526. Authored-by: Takeshi Yamamuro <[email protected]> Signed-off-by: HyukjinKwon <[email protected]> (cherry picked from commit 42f496f) Signed-off-by: HyukjinKwon <[email protected]>
| } | ||
| } | ||
|
|
||
| test("check outputs of expression examples") { |
There was a problem hiding this comment.
Hi, @maropu this test case has a minor problem when running individually for machines in different timezones: expressions such as FromUnixTime are timezone aware.
In SQLQuerySuite, the timezone is set explicitly in org.apache.spark.sql.QueryTest, however ExpressionInfoSuite doesn't set timezone, thus fails this test case.
There was a problem hiding this comment.
@HyukjinKwon and @maropu thanks for the information, I was checking an older version of Spark 3.0
5 participants
What changes were proposed in this pull request?
This PR intends to add a new test suite for
ExpressionInfo. Major changes are as follows;ExpressionInfoSuiteExpressionInfoSuiteExpressionInfo-related tests fromUDFSuitetoExpressionInfoSuiteSQLQuerySuitetoExpressionInfoSuiteExpressionInfoSuite(followup of [SPARK-31429][SQL][DOC] Automatically generates a SQL document for built-in functions #28224)Why are the changes needed?
To improve test suites/coverage.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Added tests.