-
Notifications
You must be signed in to change notification settings - Fork 63
add conformance reporting #1333
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
add conformance reporting #1333
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## partiql-plugin-impl #1333 +/- ##
======================================================
Coverage ? 49.27%
Complexity ? 1046
======================================================
Files ? 166
Lines ? 13395
Branches ? 2504
======================================================
Hits ? 6600
Misses ? 6138
Partials ? 657
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
test/partiql-tests-runner/src/main/kotlin/org/partiql/runner/ConformanceComparison.kt
Outdated
Show resolved
Hide resolved
test/partiql-tests-runner/src/main/kotlin/org/partiql/runner/ConformanceComparison.kt
Outdated
Show resolved
Hide resolved
test/partiql-tests-runner/src/main/kotlin/org/partiql/runner/ConformanceComparison.kt
Outdated
Show resolved
Hide resolved
test/partiql-tests-runner/src/main/kotlin/org/partiql/runner/ConformanceComparison.kt
Outdated
Show resolved
Hide resolved
test/partiql-tests-runner/src/main/kotlin/org/partiql/runner/ConformanceComparison.kt
Outdated
Show resolved
Hide resolved
…onformanceComparison.kt Co-authored-by: Alan Cai <[email protected]>
…onformanceComparison.kt Co-authored-by: Alan Cai <[email protected]>
…onformanceComparison.kt Co-authored-by: Alan Cai <[email protected]>
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.
Looks good. Only had a few minor comments. FYI the full list of conformance tests failing in one engine and passing in another can also be found in the CI summary https://github.com/yliuuuu/partiql-lang-kotlin/actions/runs/7483462300/attempts/1#summary-20368831620. From the docs, it appears the limit for the summary is 1MiB:
Job summaries are isolated between steps and each step is restricted to a maximum size of 1MiB.
That size limit is a lot larger than the GH comment size restriction but could be an issue if we were to add more engines or tests to the comparison report. As we get more tests passing in the eval engine, the comparison report size should decrease, so maybe it's not an issue for now.
Relevant Issues
Description
Update: We can now access the full report from Github Action run. In both the step summary and the produced artifact. See link.
For an example comment message, see: yliuuuu#15 (comment)
Note: For the new eval engine: there is one test (
path expression with ambiguous table alias (lowercase), compileOption: LEGACY
) that is showing up in Success-in-previous-but-failed-in-current and failed-in-previous-but-success-in-current. I suspect that this issue is caused by test cases with duplicated name but different query, but have not yet looked into this.Other Information
Updated Unreleased Section in CHANGELOG: [YES/NO]
Any backward-incompatible changes? [YES/NO]
Any new external dependencies? [YES/NO]
Do your changes comply with the Contributing Guidelines
and Code Style Guidelines? [YES/NO]
Yes.
License Information
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.