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

test: Boost collection test coverage #183

Merged
merged 14 commits into from
Feb 11, 2022

Conversation

AndrewSisley
Copy link
Contributor

Closes #182

No requirement for this to be added to the release

@codecov
Copy link

codecov bot commented Feb 7, 2022

Codecov Report

Merging #183 (ccc60ae) into develop (a650358) will increase coverage by 0.41%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #183      +/-   ##
===========================================
+ Coverage    57.01%   57.43%   +0.41%     
===========================================
  Files           98       98              
  Lines         9656     9643      -13     
===========================================
+ Hits          5505     5538      +33     
+ Misses        3523     3491      -32     
+ Partials       628      614      -14     
Impacted Files Coverage Δ
db/collection.go 52.58% <100.00%> (+6.63%) ⬆️
query/graphql/planner/create.go 71.15% <0.00%> (+5.76%) ⬆️

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.

Looks good, too aggresively removing some code those, check comments :)

client/core.go Outdated Show resolved Hide resolved
db/collection.go Show resolved Hide resolved
db/collection.go Outdated Show resolved Hide resolved
db/collection.go Outdated Show resolved Hide resolved
db/collection.go Outdated Show resolved Hide resolved
db/collection.go Outdated Show resolved Hide resolved
db/collection.go Outdated Show resolved Hide resolved
@jsimnz
Copy link
Member

jsimnz commented Feb 9, 2022

Prob need to rebase and make sure theres no issues, as the p2p code changes/uses some stuff here.

@AndrewSisley AndrewSisley force-pushed the sisley/test/I182-collection-code-cov branch from 8cb9e0d to ccc60ae Compare February 9, 2022 19:37
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.

Last item to keep, on the newCollection. Added a comment regarding Exists too, lmk

db/collection.go Show resolved Hide resolved
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, up to you if you want to keep/remove that line in newCollection, added my opinion. Other than that, looks golden. 👍

@AndrewSisley AndrewSisley merged commit 5799ca7 into develop Feb 11, 2022
@AndrewSisley AndrewSisley deleted the sisley/test/I182-collection-code-cov branch February 11, 2022 14:19
shahzadlone pushed a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
* Remove comments

* Remove unused case (unsupported type)

* Add dupilcate doc creation test

* Remove commented out code

* Add test for non-existant schema

* Add test for empty collection name

* Remove impossible if-clause

* Add test for collection already existing

* Remove duplicate check

Is identical to the next check - fields == 0

* Add tests for schema with no fields

* Add test for collection with no name

* Add key field tests

* Remove impossible check

Sequence code does not permit this to be true, and this private function is only called with an input from that sequence code

* Add tests for field property validation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Collection.go test coverage is low
2 participants