Initial conformance tests for graph query #1104
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #1104 +/- ##
============================================
+ Coverage 74.89% 74.92% +0.02%
- Complexity 2449 2454 +5
============================================
Files 255 255
Lines 18959 18961 +2
Branches 3456 3457 +1
============================================
+ Hits 14200 14206 +6
+ Misses 3691 3690 -1
+ Partials 1068 1065 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
Conformance comparison report
Number passing in both: 5204 Number failing in both: 438 Number passing in Base (86e184a) but now fail: 0 Number failing in Base (86e184a) but now pass: 2 The following test(s) were previously failing but now pass. Before merging, confirm they are intended to pass: Click here to see
|
| @ParameterizedTest | ||
| @ArgumentsSource(Directionality::class) | ||
| fun testDirectionality(qr: Pair<String, String>) { | ||
| testGraphQueries(sessionDirectionality, qr) | ||
| } | ||
|
|
||
| class Directionality : ArgumentsProviderBase() { |
There was a problem hiding this comment.
Will these tests still remain in partiql-lang-kotlin after partiql/partiql-tests#91 is merged into partiql-tests? Seems like a duplication of tests.
Or perhaps are we waiting for the Kotlin conformance test runner improvements that we spoke about in #1096 before relying just on partiql-tests?
There was a problem hiding this comment.
I'll follow your advice here.
For tests that have been previously ported from kotlin to conformance, have originals always been removed? My understanding was that in many cases they remained within the unit tests. I, indeed, feel this can be left to settle until the runner improvements or even some larger re-assessment of unit tests vs conformance tests relationship.
There was a problem hiding this comment.
For tests that have been previously ported from kotlin to conformance, have originals always been removed?
Tests that were part of pts were removed (#910). The other tests haven't yet been removed yet (e.g. porting from EvaluatorTestSuite.kt.
Agree with the point that the test runner would need improvements in running the tests and modeling passing/failing tests before we go through with deleting the other tests.
partiql-lang/src/test/kotlin/org/partiql/lang/eval/EvaluatingCompilerGraphMatchTests.kt
Show resolved
Hide resolved
Start on conformance tests for graph query. These conformance tests are ported from some of the unit tests in EvaluatingCompilerGraphMatchTests in partiql-lang-kotlin, partiql/partiql-lang-kotlin#1104
With partiql-lang-kotlin, 2 of them now suceed and the other fails for a better reason. These fixes stem from exploring the purported conformance regression in partiql/partiql-lang-kotlin#1104. (The proximate cause of (wrongly) detecting the regression there was probably that 0007.ion contained two different tests under the same name.)
Accounting for the merge of partiql-tests #91.
This makes it possible to re-enable 6 tests that were commented out in #1104, because the deduplication of path bindings was not happening. Interestingly, 2 of those tests initially had wrong outcomes specified, now corrected. (Being a case where the deduplication should not apply.)
Adjustments for running graph conformance tests in partiql-tests (as needed for loading external graphs into evaluation environments) and the tests themselves.
The tests come in two variations: unit tests and conformance tests in
partiql-testssubmodule.Other Information
Updated Unreleased Section in CHANGELOG: NO
Any backward-incompatible changes? NO
Any new external dependencies? NO
License Information
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.