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: Error when attempting to insert value into relationship field #632

Merged
merged 2 commits into from
Jul 14, 2022

Conversation

AndrewSisley
Copy link
Contributor

@AndrewSisley AndrewSisley commented Jul 13, 2022

Relevant issue(s)

Resolves #105

Description

Errors when attempting to insert value into relationship field instead of permitting the save (and then failing to read the relationship on query).

Also reworks the newish collection test framework - splitting it off from the query framework allowing both to be simplified. This does mean that collection tests will not go through the change-detector, and will only run against badger-IM - I don't see that as loss atm, but please discuss if you think otherwise.

@AndrewSisley AndrewSisley added bug Something isn't working area/collections Related to the collections system action/no-benchmark Skips the action that runs the benchmark. labels Jul 13, 2022
@AndrewSisley AndrewSisley added this to the DefraDB v0.3 milestone Jul 13, 2022
@AndrewSisley AndrewSisley requested a review from a team July 13, 2022 16:16
@AndrewSisley AndrewSisley self-assigned this Jul 13, 2022
"github.com/stretchr/testify/assert"
)

func TestSaveErrorsGivenUnknownField(t *testing.T) {
func TestUpdateSaveErrorsGivenUnknownField(t *testing.T) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Test also got renamed to distinguish it from the Create tests, I lazily showed this into the (first?) commit

@AndrewSisley AndrewSisley force-pushed the sisley/fix/I105-one-many-insert branch from adfd73c to 61d96ea Compare July 13, 2022 16:26
Comment on lines 24 to 29
[]byte(
`{
"Name": "Painted House",
"Author": "ValueDoesntMatter"
}`,
),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
[]byte(
`{
"Name": "Painted House",
"Author": "ValueDoesntMatter"
}`,
),
[]byte(
`{
"Name": "Painted House",
"Author": "ValueDoesntMatter"
}`,
),

Copy link
Member

@shahzadlone shahzadlone left a comment

Choose a reason for hiding this comment

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

Alt Text

Decouples them from thequery integration tests, allowing for a more focused interface and the removal of some complication from the query tests too.
@AndrewSisley AndrewSisley force-pushed the sisley/fix/I105-one-many-insert branch from 61d96ea to a995cad Compare July 14, 2022 19:51
@AndrewSisley AndrewSisley force-pushed the sisley/fix/I105-one-many-insert branch from a995cad to 8cf0cb0 Compare July 14, 2022 19:52
@AndrewSisley AndrewSisley merged commit cce2f17 into develop Jul 14, 2022
@AndrewSisley AndrewSisley deleted the sisley/fix/I105-one-many-insert branch July 14, 2022 20:05
shahzadlone pushed a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
…ourcenetwork#632)

* Simplify collection integration tests

Decouples them from thequery integration tests, allowing for a more focused interface and the removal of some complication from the query tests too.

* Error when trying to save value to relation field
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/collections Related to the collections system bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test case results in panic in fetcher.Close
2 participants