Skip to content

Commit ddafe0c

Browse files
authored
docs: rename SQL File Tests to Comet SQL Tests and Scala tests to Comet Scala Tests (#4108)
1 parent c7d97db commit ddafe0c

6 files changed

Lines changed: 35 additions & 35 deletions

File tree

.claude/skills/audit-comet-expression/SKILL.md

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ This audit covers:
1313
1. Spark implementation across versions 3.4.3, 3.5.8, and 4.0.1
1414
2. Comet Scala serde implementation
1515
3. Comet Rust / DataFusion implementation
16-
4. Existing test coverage (SQL file tests and Scala tests)
16+
4. Existing test coverage (Comet SQL Tests and Comet Scala Tests)
1717
5. Gap analysis and test recommendations
1818

1919
---
@@ -161,7 +161,7 @@ Read the Rust implementation and check:
161161

162162
## Step 4: Locate Existing Comet Tests
163163

164-
### SQL file tests
164+
### Comet SQL Tests
165165

166166
```bash
167167
# Find SQL test files for this expression
@@ -179,13 +179,13 @@ Read every SQL test file found and list:
179179
- Query modes used (`query`, `spark_answer_only`, `tolerance`, `ignore`, `expect_error`)
180180
- Any ConfigMatrix directives
181181

182-
### Scala tests
182+
### Comet Scala Tests
183183

184184
```bash
185185
grep -r "$ARGUMENTS" spark/src/test/scala/ --include="*.scala" -l
186186
```
187187

188-
Read the relevant Scala test files and list:
188+
Read the relevant Comet Scala Tests and list:
189189

190190
- Input types covered
191191
- Edge cases exercised
@@ -201,7 +201,7 @@ Compare the Spark test coverage (Step 2) against the Comet test coverage (Step 4
201201

202202
For each of the following dimensions, note whether it is covered in Comet tests or missing:
203203

204-
| Dimension | Spark tests it | Comet SQL test | Comet Scala test | Gap? |
204+
| Dimension | Spark tests it | Comet SQL Test | Comet Scala Test | Gap? |
205205
| ------------------------------------------------------------------------------------------------------ | -------------- | -------------- | ---------------- | ---- |
206206
| Column reference argument(s) | | | | |
207207
| Literal argument(s) | | | | |
@@ -256,13 +256,13 @@ After presenting the gap analysis, ask the user:
256256
>
257257
> - [list each missing test case]
258258
>
259-
> I can add them as SQL file tests in `spark/src/test/resources/sql-tests/expressions/<category>/$ARGUMENTS.sql`
260-
> (or as Scala tests in `CometExpressionSuite` for cases that require programmatic setup).
259+
> I can add them as Comet SQL Tests in `spark/src/test/resources/sql-tests/expressions/<category>/$ARGUMENTS.sql`
260+
> (or as Comet Scala Tests in `CometExpressionSuite` for cases that require programmatic setup).
261261
262-
If the user says yes, implement the missing tests following the SQL file test format described in
263-
`docs/source/contributor-guide/sql-file-tests.md`. Prefer SQL file tests over Scala tests.
262+
If the user says yes, implement the missing tests following the Comet SQL Tests format described in
263+
`docs/source/contributor-guide/sql-file-tests.md`. Prefer Comet SQL Tests over Comet Scala Tests.
264264

265-
### SQL file test template
265+
### Comet SQL Tests template
266266

267267
```sql
268268
-- Licensed to the Apache Software Foundation (ASF) under one

.claude/skills/review-comet-pr/SKILL.md

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -138,15 +138,15 @@ Location: `native/spark-expr/src/`
138138
- [ ] No panics. Use `Result` types.
139139
- [ ] Efficient array operations (avoid row-by-row)
140140

141-
#### Tests - Prefer SQL File-Based Framework
141+
#### Tests - Prefer Comet SQL Tests
142142

143-
**Expression tests should use the SQL file-based framework (`CometSqlFileTestSuite`) where possible.** This framework automatically runs each query through both Spark and Comet and compares results. No Scala code is needed. Only fall back to Scala tests in `CometExpressionSuite` when the SQL framework cannot express the test. Examples include complex `DataFrame` setup, programmatic data generation, or non-expression tests.
143+
**Expression tests should use Comet SQL Tests (`CometSqlFileTestSuite`) where possible.** This framework automatically runs each query through both Spark and Comet and compares results. No Scala code is needed. Only fall back to Comet Scala Tests in `CometExpressionSuite` when Comet SQL Tests cannot express the test. Examples include complex `DataFrame` setup, programmatic data generation, or non-expression tests.
144144

145-
**SQL file test location:** `spark/src/test/resources/sql-tests/expressions/<category>/`
145+
**Comet SQL Test location:** `spark/src/test/resources/sql-tests/expressions/<category>/`
146146

147147
Categories include: `aggregate/`, `array/`, `string/`, `math/`, `struct/`, `map/`, `datetime/`, `hash/`, etc.
148148

149-
**SQL file structure:**
149+
**Comet SQL Test structure:**
150150

151151
```sql
152152
-- Create test data
@@ -181,10 +181,10 @@ query ignore(https://github.com/apache/datafusion-comet/issues/NNNN)
181181
SELECT known_buggy_expr(v) FROM test_table
182182
```
183183

184-
**Running SQL file tests:**
184+
**Running Comet SQL Tests:**
185185

186186
```bash
187-
# All SQL file tests
187+
# All Comet SQL Tests
188188
./mvnw test -Dsuites="org.apache.comet.CometSqlFileTestSuite" -Dtest=none
189189

190190
# Specific test file (substring match)
@@ -199,7 +199,7 @@ SELECT known_buggy_expr(v) FROM test_table
199199
- [ ] Both literal values and column references tested (they use different code paths)
200200
- [ ] For timestamp/datetime expressions, timezone handling is tested (e.g., UTC, non-UTC session timezone, timestamps with and without timezone)
201201
- [ ] One expression per SQL file for easier debugging
202-
- [ ] If using Scala tests instead, literal tests MUST disable constant folding:
202+
- [ ] If using Comet Scala Tests instead, literal tests MUST disable constant folding:
203203
```scala
204204
withSQLConf(SQLConf.OPTIMIZER_EXCLUDED_RULES.key ->
205205
"org.apache.spark.sql.catalyst.optimizer.ConstantFolding") {
@@ -256,7 +256,7 @@ If the PR adds a new expression or operator but does not update the relevant doc
256256
1. **Incomplete type support**: Spark expression supports types not handled in PR
257257
2. **Missing edge cases**: Null, overflow, empty string, negative values
258258
3. **Wrong return type**: Return type must match Spark exactly
259-
4. **Tests in wrong framework**: Expression tests should use the SQL file-based framework (`CometSqlFileTestSuite`) rather than adding to Scala test suites like `CometExpressionSuite`. Suggest migration if the PR adds Scala tests for expressions that could use SQL files instead.
259+
4. **Tests in wrong framework**: Expression tests should use Comet SQL Tests (`CometSqlFileTestSuite`) rather than adding to Comet Scala Tests like `CometExpressionSuite`. Suggest migration if the PR adds Comet Scala Tests for expressions that could use Comet SQL Tests instead.
260260
5. **Stale native code**: PR might need `./mvnw install -pl common -DskipTests`
261261
6. **Missing `getSupportLevel`**: Edge cases should be marked as `Incompatible`
262262

docs/source/contributor-guide/adding_a_new_expression.md

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -273,9 +273,9 @@ override def getUnsupportedReasons(): Seq[String] = Seq(
273273

274274
#### Adding Spark-side Tests for the New Expression
275275

276-
It is important to verify that the new expression is correctly recognized by the native execution engine and matches the expected Spark behavior. The preferred way to add test coverage is to write a SQL test file using the SQL file test framework. This approach is simpler than writing Scala test code and makes it easy to cover many input combinations and edge cases.
276+
It is important to verify that the new expression is correctly recognized by the native execution engine and matches the expected Spark behavior. The preferred way to add test coverage is to write a Comet SQL Test. This approach is simpler than writing Comet Scala Tests and makes it easy to cover many input combinations and edge cases.
277277

278-
##### Writing a SQL test file
278+
##### Writing a Comet SQL Test
279279

280280
Create a `.sql` file under the appropriate subdirectory in `spark/src/test/resources/sql-tests/expressions/` (e.g., `string/`, `math/`, `array/`). The file should create a table with test data, then run queries that exercise the expression. Here is an example for the `unhex` expression:
281281

@@ -313,17 +313,17 @@ Run the test with:
313313
./mvnw test -Dsuites="org.apache.comet.CometSqlFileTestSuite unhex" -Dtest=none
314314
```
315315

316-
For full documentation on the test file formatincluding directives like `ConfigMatrix`, query modes like `spark_answer_only` and `tolerance`, handling known bugs with `ignore(...)`, and tips for writing thorough testssee the [SQL File Tests](sql-file-tests.md) guide.
316+
For full documentation on the test file format, including directives like `ConfigMatrix`, query modes like `spark_answer_only` and `tolerance`, handling known bugs with `ignore(...)`, and tips for writing thorough tests, see the [Comet SQL Tests](sql-file-tests.md) guide.
317317

318318
##### Tips
319319

320-
- **Cover both column references and literals.** Comet often uses different code paths for each. The SQL file test suite automatically disables constant folding, so all-literal queries are evaluated natively.
320+
- **Cover both column references and literals.** Comet often uses different code paths for each. The Comet SQL Tests suite automatically disables constant folding, so all-literal queries are evaluated natively.
321321
- **Include edge cases** such as `NULL`, empty strings, boundary values, `NaN`, and multibyte UTF-8 characters.
322322
- **Keep one file per expression** to make failures easy to locate.
323323

324-
##### Scala tests (alternative)
324+
##### Comet Scala Tests (alternative)
325325

326-
For cases that require programmatic setup or custom assertions beyond what SQL files support, you can also add Scala test cases in `CometExpressionSuite` using the `checkSparkAnswerAndOperator` method:
326+
For cases that require programmatic setup or custom assertions beyond what SQL files support, you can also add Comet Scala Tests in `CometExpressionSuite` using the `checkSparkAnswerAndOperator` method:
327327

328328
```scala
329329
test("unhex") {
@@ -347,7 +347,7 @@ test("unhex") {
347347
}
348348
```
349349

350-
When writing Scala tests with literal values (e.g., `SELECT my_func('literal')`), Spark's constant folding optimizer may evaluate the expression at planning time, bypassing Comet. To prevent this, disable constant folding:
350+
When writing Comet Scala Tests with literal values (e.g., `SELECT my_func('literal')`), Spark's constant folding optimizer may evaluate the expression at planning time, bypassing Comet. To prevent this, disable constant folding:
351351

352352
```scala
353353
test("my_func with literals") {

docs/source/contributor-guide/development.md

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -281,14 +281,14 @@ size limit to 16 MB.
281281
First make sure to install the Rust plugin in CLion or you can use the dedicated Rust IDE: RustRover.
282282
After that you can open the project in CLion. The IDE should automatically detect the project structure and import as a Cargo project.
283283

284-
### SQL file tests (recommended for expressions)
284+
### Comet SQL Tests (recommended for expressions)
285285

286-
For testing expressions and operators, prefer using SQL file tests over writing Scala test
287-
code. SQL file tests are plain `.sql` files that are automatically discovered and executed --
286+
For testing expressions and operators, prefer using Comet SQL Tests over writing Comet Scala
287+
Tests. Comet SQL Tests are plain `.sql` files that are automatically discovered and executed:
288288
no Scala code to write, and no recompilation needed when tests change. This makes it easy to
289289
iterate quickly and to get good coverage of edge cases and argument combinations.
290290

291-
See the [SQL File Tests](sql-file-tests) guide for the full documentation on how to write
291+
See the [Comet SQL Tests](sql-file-tests) guide for the full documentation on how to write
292292
and run these tests.
293293

294294
### Running Tests in IDEA
@@ -444,7 +444,7 @@ Comet's CI does not automatically discover test suites. Instead, test suites are
444444
in the GitHub Actions workflow files so they can be grouped by category and run as separate parallel
445445
jobs. This reduces overall CI time.
446446

447-
If you add a new Scala test suite, you must add it to the `suite` matrix in **both** workflow files:
447+
If you add a new Comet Scala Test suite, you must add it to the `suite` matrix in **both** workflow files:
448448

449449
- `.github/workflows/pr_build_linux.yml`
450450
- `.github/workflows/pr_build_macos.yml`
@@ -471,7 +471,7 @@ Choose the group that best matches the area your test covers:
471471
| `parquet` | Parquet read/write and native reader tests |
472472
| `csv` | CSV native read tests |
473473
| `exec` | Execution operators, joins, aggregates, plan rules, TPC-\* |
474-
| `expressions` | Expression evaluation, casts, and SQL file tests |
474+
| `expressions` | Expression evaluation, casts, and Comet SQL Tests |
475475
| `sql` | SQL-level behavior tests |
476476

477477
**Important:** The suite lists in both workflow files must stay in sync. A separate CI check

docs/source/contributor-guide/index.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,9 @@ Adding a New Expression <adding_a_new_expression>
3636
Supported Spark Expressions <spark_expressions_support>
3737
Tracing <tracing>
3838
Profiling <profiling>
39+
Comet SQL Tests <sql-file-tests.md>
3940
Spark SQL Tests <spark-sql-tests.md>
4041
Iceberg Spark Tests <iceberg-spark-tests.md>
41-
SQL File Tests <sql-file-tests.md>
4242
Bug Triage <bug_triage>
4343
Roadmap <roadmap.md>
4444
Release Process <release_process>

docs/source/contributor-guide/sql-file-tests.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,15 @@ specific language governing permissions and limitations
1717
under the License.
1818
-->
1919

20-
# SQL File Tests
20+
# Comet SQL Tests
2121

2222
`CometSqlFileTestSuite` is a test suite that automatically discovers `.sql` test files and
2323
runs each query through both Spark and Comet, comparing results. This provides a lightweight
24-
way to add expression and operator test coverage without writing Scala test code.
24+
way to add expression and operator test coverage without writing Comet Scala Tests.
2525

2626
## Running the tests
2727

28-
Run all SQL file tests:
28+
Run all Comet SQL Tests:
2929

3030
```shell
3131
./mvnw test -Dsuites="org.apache.comet.CometSqlFileTestSuite" -Dtest=none

0 commit comments

Comments
 (0)