sunchao opened a new pull request, #5417:
URL: https://github.com/apache/datafusion-comet/pull/5417

   ## Why are the changes needed?
   
   The suite added in #5329 checks outcomes only after proving that the default 
and forced-fallback legs exercised different execution paths. However, an 
unprovable comparison currently becomes `SKIPPED-VACUOUS`, and the final 
assertion ignores those entries. The suite can therefore pass with `pass=0 
fail=0 vacuous=25`, or stay green after losing an individual witness such as 
`AddMonths`.
   
   Unexpected coverage loss in this fixed regression corpus should fail the 
test instead of requiring someone to notice a changed counter in the logs. This 
is test-coverage enforcement, not a product behavior change; other suites also 
guard the historical `AddMonths` bug.
   
   ### Which issue does this PR close?
   
   Follow-up to #5329 and its original issue, #5328. No new issue is closed.
   
   ## What changes were proposed in this PR?
   
   - Classify unexpected opaque plans, non-native defaults, and ineffective 
fallback toggles as `FAIL-BIND`, which reaches the existing final assertion.
   - Permit only `values / StringTranslate / not-native-by-default` as the 
known vacuous exception. If `StringTranslate` gains native support, it may pass 
normally; the test does not require a fixed skip count.
   - Keep verdict counters exclusive and update the suite documentation and 
assertion message to cover binding failures.
   - Add ten regression scenarios in the same suite, covering all binding-gate 
branches and the expression, section, and reason boundaries of the exception.
   
   Only `CometFallbackInvarianceSuite.scala` changes. No product code or CI 
workflow changes.
   
   ## How was this PR tested?
   
   Rebuilt the native library from this upstream checkout with `cargo build 
--locked --offline`, then ran the focused root Maven command on Spark 4.1.3 
with JDK 17:
   
   ```sh
   ./mvnw -o test -Dtest=none 
-Dsuites=org.apache.comet.CometFallbackInvarianceSuite
   ```
   
   All **6 ScalaTest tests passed**, including the ten-scenario guard test in 
each of the three shuffle/C2R variants. The new guard test also failed in all 
three variants when compiled against the old implementation, before the binding 
fix.
   
   Negative controls used the same upstream runtime in isolated JVMs. These 
counters were observed in every variant:
   
   | Configuration | Pass | Fail | Vacuous | Result |
   | --- | ---: | ---: | ---: | --- |
   | Healthy baseline | 24 | 0 | 1 | Passed |
   | `spark.comet.exec.project.enabled=false` | 0 | 24 | 1 | Failed as expected 
|
   | `spark.comet.expression.AddMonths.enabled=false` | 23 | 1 | 1 | Failed as 
expected |
   
   Spotless, scalastyle, and `git diff --check` passed. Local runtime 
validation is Spark 4.1.3 only; the remaining Spark profiles are left to CI.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to