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.

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

fix: Return empty array if no values found #223

Merged
merged 3 commits into from
Feb 21, 2022

Conversation

AndrewSisley
Copy link
Contributor

@AndrewSisley AndrewSisley commented Feb 18, 2022

Closes #222

PR includes 3 separate fixes, probably easier to review commit by commit.

@AndrewSisley AndrewSisley added the area/testing Related to any test or testing suite label Feb 18, 2022
@AndrewSisley AndrewSisley self-assigned this Feb 18, 2022
@codecov
Copy link

codecov bot commented Feb 18, 2022

Codecov Report

Merging #223 (67a1f02) into develop (270fba2) will increase coverage by 0.02%.
The diff coverage is 80.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #223      +/-   ##
===========================================
+ Coverage    57.90%   57.93%   +0.02%     
===========================================
  Files           99       99              
  Lines         9571     9582      +11     
===========================================
+ Hits          5542     5551       +9     
- Misses        3413     3414       +1     
- Partials       616      617       +1     
Impacted Files Coverage Δ
db/tests/utils.go 59.36% <75.00%> (+0.59%) ⬆️
query/graphql/planner/planner.go 72.79% <100.00%> (+0.31%) ⬆️

Copy link
Contributor

@orpheuslummis orpheuslummis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[]map[string]interface{}{} is the zero value of a slice which is nil. It seems a matter of preference as which to choose, and I'd recommend consistency with the rest.

assertQueryResults wasn't working properly - i.e. not doing the assertion -- for empty values. This fixes it. 👍

@AndrewSisley
Copy link
Contributor Author

[]map[string]interface{}{} is the zero value of a slice which is nil. It seems a matter of preference as which to choose, and I'd recommend consistency with the rest.

There is some difference as test tests failed before I changed the expected values - might be due to Golang wierd (nil)(type) != nil stuff - looking deeper into the prod code, as the transactional results don't seem to care whether the expected is empty or nil

@AndrewSisley
Copy link
Contributor Author

Expected to add quick assert, ended up finding a bug in prod and test code... Code should make a little more sense now.

@AndrewSisley AndrewSisley changed the title Assert nil/empty values are equal Return empty array if no values found Feb 18, 2022
@AndrewSisley AndrewSisley changed the title Return empty array if no values found fix: Return empty array if no values found Feb 18, 2022
@AndrewSisley AndrewSisley added area/query Related to the query component bug Something isn't working labels Feb 18, 2022
Copy link
Member

@jsimnz jsimnz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@AndrewSisley AndrewSisley merged commit 3d74e8c into develop Feb 21, 2022
@AndrewSisley AndrewSisley deleted the sisley/test/I222-test-not-nil branch February 21, 2022 15:49
shahzadlone pushed a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
* Return empty if no values found

* Assert nil/empty values are equal

* Correctly handle errors in transaction tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/query Related to the query component area/testing Related to any test or testing suite bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Assert empty results (and not nil) in integration tests when expecting results.len==0
3 participants