fix: Make new comparison tests parseable #772
Conversation
EpsilonPrime
requested review from
jacques-n,
cpcloud,
westonpace and
vbarua
as code owners
|
ACTION NEEDED Substrait follows the Conventional Commits The PR title and description are used as the merge commit message. Please update your PR title and description to match the specification. |
EpsilonPrime
changed the title
| @@ -1,6 +1,7 @@ | |||
| ### SUBSTRAIT_SCALAR_TEST: v1.0 | |||
| ### SUBSTRAIT_INCLUDE: '/extensions/functions_comparison.yaml' | |||
|
|
|||
| # complete | |||
There was a problem hiding this comment.
Does our grammar require description? Did we make that deliberately make that decision? (can't remember)
There was a problem hiding this comment.
This is test group name/description. should it be optional?
There was a problem hiding this comment.
It is not optional, hence the need to fix the test.
There was a problem hiding this comment.
I was proposing if we should make it optional?
- all tests without group description, will assign a default name something like
default_<func>_tests - OR add code to fail the pre-commit/PR workflow if there is no group description
There was a problem hiding this comment.
I tried making it optional in #769 but according to David there are issues downstream with that.
No description provided.