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 test for successful one-one create mutation #1215

Merged
merged 4 commits into from
Mar 21, 2023

Conversation

AndrewSisley
Copy link
Contributor

Relevant issue(s)

Resolves #1212

Description

Adds a test for a successful one-one create mutation. Only failures and partial creation (no linkage) were tested before, possibly due to limitations in the old test framework.

Also migrates the existing tests to the new framework, and links the documentation tests to new created github issues.

Gap was spotted whilst adding examples to the user guides.

Might as well migrate the 4 existing tests before going through the hassle of adding new ones in a different (new) framework.
@AndrewSisley AndrewSisley added area/query Related to the query component area/testing Related to any test or testing suite labels Mar 21, 2023
@AndrewSisley AndrewSisley added this to the DefraDB v0.5 milestone Mar 21, 2023
@AndrewSisley AndrewSisley requested a review from a team March 21, 2023 22:08
@AndrewSisley AndrewSisley self-assigned this Mar 21, 2023
@codecov
Copy link

codecov bot commented Mar 21, 2023

Codecov Report

Merging #1215 (53791db) into develop (f0e2036) will increase coverage by 0.08%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1215      +/-   ##
===========================================
+ Coverage    68.57%   68.65%   +0.08%     
===========================================
  Files          181      181              
  Lines        17125    17125              
===========================================
+ Hits         11743    11758      +15     
+ Misses        4424     4413      -11     
+ Partials       958      954       -4     

see 9 files with indirect coverage changes

@source-devs
Copy link

Benchmark Results

Summary

  • 0 Benchmarks successfully compared.
  • 0 Benchmarks were ✅ Better.
  • 0 Benchmarks were ❌ Worse .
  • 0 Benchmarks were ✨ Unchanged.
✅ See Better Results...
time/opdelta
 
❌ See Worse Results...
time/opdelta
 
✨ See Unchanged Results...
time/opdelta
 
🐋 See Full Results...

@AndrewSisley AndrewSisley force-pushed the sisley/refactor/I1212-one-one-create-tests branch from e9b2904 to 53791db Compare March 21, 2023 22:20
@source-devs
Copy link

Benchmark Results

Summary

  • 0 Benchmarks successfully compared.
  • 0 Benchmarks were ✅ Better.
  • 0 Benchmarks were ❌ Worse .
  • 0 Benchmarks were ✨ Unchanged.
✅ See Better Results...
time/opdelta
 
❌ See Worse Results...
time/opdelta
 
✨ See Unchanged Results...
time/opdelta
 
🐋 See Full Results...

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. Thanks for catching!

@jsimnz jsimnz added the action/no-benchmark Skips the action that runs the benchmark. label Mar 21, 2023
@AndrewSisley AndrewSisley merged commit 56b89d7 into develop Mar 21, 2023
@AndrewSisley AndrewSisley deleted the sisley/refactor/I1212-one-one-create-tests branch March 21, 2023 22:33
Comment on lines +21 to +22
// This test documents incorrect behaviour. It should be possible to create author then book,
// linking in the second create step (like in [TestMutationCreateOneToOne]).
Copy link
Collaborator

Choose a reason for hiding this comment

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

question: Can you help me understand why this is incorrect behaviour? I feel like both this one and TestMutationCreateOneToOneNoChild should fail for the same reason that their assigned relationship doesn't exist.

Copy link
Contributor Author

@AndrewSisley AndrewSisley Mar 21, 2023

Choose a reason for hiding this comment

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

The problem is with the error that is returned. author_id is flagged as not existing whereas it should be seen as existing - the test should create the book (until TestMutationCreateOneToOneNoChild is fixed to error correctly when trying to link to a doc that doesnt exist).

When/whilst the bug is fixed/being-fixed, this test should be amended/duplicated so that the author it is pointed to actually exists prior to book creation.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Alright I understand now thanks.

shahzadlone pushed a commit that referenced this pull request Apr 13, 2023
* Migrate one-one tests to new framework

Might as well migrate the 4 existing tests before going through the hassle of adding new ones in a different (new) framework.

* Document documentation test

* Document documentation test

* Add one-one create mutation test
shahzadlone pushed a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
…1215)

* Migrate one-one tests to new framework

Might as well migrate the 4 existing tests before going through the hassle of adding new ones in a different (new) framework.

* Document documentation test

* Document documentation test

* Add one-one create mutation test
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/query Related to the query component area/testing Related to any test or testing suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No tests for one-one create of linked documents
4 participants