Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Move operator tests to logic tests #226

Closed
1 of 2 tasks
asubiotto opened this issue Oct 7, 2022 · 11 comments · Fixed by #260
Closed
1 of 2 tasks

Move operator tests to logic tests #226

asubiotto opened this issue Oct 7, 2022 · 11 comments · Fixed by #260
Labels
good first issue Good for newcomers Stale

Comments

@asubiotto
Copy link
Member

asubiotto commented Oct 7, 2022 •
edited
Loading

A datadriven logic testing framework was recently added in #211. Slowly but surely, it would be nice to move our operator tests to this logic testing framework to promote readability and conciseness. #255 does this for the distinct operator. This issue can be closed once the following tests have been moved over:

  • aggregate_test.go
  • filter_test.go
@asubiotto asubiotto added the good first issue Good for newcomers label Oct 7, 2022
@the-writer-dev
Copy link

@asubiotto Can you assign this to me?

@asubiotto
Copy link
Member Author

Sure!

@the-writer-dev
Copy link

@asubiotto Thanks!!!

@palash25
Copy link
Contributor

palash25 commented Nov 4, 2022

hi @bitethecode are you still working on this? if not then i would like to pick this up if that's ok with you

@asubiotto
Copy link
Member Author

Hi @palash25, are you still interested in working on this? I can reassign to you given that we haven't heard back.

@the-writer-dev
Copy link

Sorry for being late here. I tried to figure it out but it's too complicated to me. You can unassign!

@asubiotto asubiotto assigned palash25 and unassigned the-writer-dev Nov 22, 2022
@asubiotto
Copy link
Member Author

No problem! Thanks for giving it a shot. @palash25 assigned to you. Feel free to use #225 as a guide.

@palash25
Copy link
Contributor

thanks for assigning i will look into it this weekend

This was referenced Nov 27, 2022
Merged
Merged
palash25 added a commit to palash25/frostdb that referenced this issue Dec 1, 2022
@palash25
Convert filter_test to logic tests
888011a 
Also adds support for NotEq and Regexp operators to the sqlparser

Issue: polarsignals#226

Signed-off-by: Palash Nigam <npalash25@gmail.com>
@palash25
Copy link
Contributor

palash25 commented Dec 2, 2022

as i have already submitted 2 PRs, i would like to be unassigned so that someone else can try to convert the remaining go tests

thorfour pushed a commit that referenced this issue Dec 3, 2022
@palash25
Convert filter_test to logic tests (#260)
7f14a4d 
Also adds support for NotEq and Regexp operators to the sqlparser

Issue: #226

Signed-off-by: Palash Nigam <npalash25@gmail.com>

Signed-off-by: Palash Nigam <npalash25@gmail.com>
@asubiotto
Copy link
Member Author

I think there are still some tests to be moved (notably the projection tests in aggregate_test.go). I believe #302 will likely address this, but reopening to track.

@asubiotto asubiotto reopened this Mar 6, 2023
@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 5, 2024

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the Stale label Jan 5, 2024
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jan 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue Good for newcomers Stale
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Convert filter_test to logic tests palash25/frostdb
3 participants
@asubiotto @palash25 @the-writer-dev