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: Add tests for default properties #611

Merged
merged 3 commits into from
Jul 11, 2022

Conversation

AndrewSisley
Copy link
Contributor

Resolves #412

Description

Adds tests for default properties. Had to change a few test cids when adding a new field to a test schema, as that affects them. Includes a failing/documentation test linked to an issue (also noted in issue).

@AndrewSisley AndrewSisley added area/testing Related to any test or testing suite code quality Related to improving code quality action/no-benchmark Skips the action that runs the benchmark. labels Jul 8, 2022
@AndrewSisley AndrewSisley added this to the DefraDB v0.3 milestone Jul 8, 2022
@AndrewSisley AndrewSisley requested a review from a team July 8, 2022 18:39
@AndrewSisley AndrewSisley self-assigned this Jul 8, 2022
Copy link
Collaborator

@fredcarle fredcarle left a comment

Choose a reason for hiding this comment

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

Approving assuming you address my concerns bellow :)


func TestQuerySimpleWithSomeDefaultValues(t *testing.T) {
test := testUtils.QueryTestCase{
Description: "...",
Copy link
Collaborator

Choose a reason for hiding this comment

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

question: did you omit the description on purpose?

Copy link
Contributor Author

@AndrewSisley AndrewSisley Jul 11, 2022

Choose a reason for hiding this comment

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

Nope I did not :)

  • desc

// A document with nil fields should be returned.
func TestQuerySimpleWithDefaultValue(t *testing.T) {
test := testUtils.QueryTestCase{
Description: "...",
Copy link
Collaborator

Choose a reason for hiding this comment

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

question: same as above.

Copy link
Contributor Author

@AndrewSisley AndrewSisley Jul 11, 2022

Choose a reason for hiding this comment

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

  • desc

Comment on lines +172 to +177
Docs: map[int][]string{
0: {
`{ }`,
},
},
Results: []map[string]interface{}{},
Copy link
Collaborator

Choose a reason for hiding this comment

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

though: The way I interpret this is that you add an empty object to the database which from the expected result tells me that an empty object will be discarded. Am I thinking about this right? Otherwise I would expect this to be the result:

{
	"Name":     nil,
	"Email":    nil,
	"Age":      nil,
	"HeightM":  nil,
	"Verified": nil,
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct - there is a comment linked to an issue regarding this at the top of the test function.

@AndrewSisley AndrewSisley force-pushed the sisley/test/I412-default-prop-tests branch 2 times, most recently from 5ccf755 to 8bec1ac Compare July 11, 2022 14:35
It is not true when a doc has been read, but when the whole span is complete
Need two string fields for an upcoming test case
@AndrewSisley AndrewSisley force-pushed the sisley/test/I412-default-prop-tests branch from 8bec1ac to aad0efc Compare July 11, 2022 14:43
@AndrewSisley AndrewSisley merged commit f9d5c0b into develop Jul 11, 2022
@AndrewSisley AndrewSisley deleted the sisley/test/I412-default-prop-tests branch July 11, 2022 14:50
shahzadlone pushed a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
* Rename variable

It is not true when a doc has been read, but when the whole span is complete

* Add email field to simple test schema

Need two string fields for an upcoming test case

* Add tests for default properties
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action/no-benchmark Skips the action that runs the benchmark. area/testing Related to any test or testing suite code quality Related to improving code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No tests for default properties
2 participants